Re: [Y2038] [PATCH v4 02/10] include: Move compat_timespec/ timeval to compat_time.h
On Thu, Mar 15, 2018 at 09:04:04AM +0100, Arnd Bergmann wrote: On Thu, Mar 15, 2018 at 3:51 AM, Deepa Dinamani wrote: On Wed, Mar 14, 2018 at 1:52 PM, Arnd Bergmann wrote: On Wed, Mar 14, 2018 at 4:50 AM, Deepa Dinamani wrote: The file arch/arm64/kernel/process.c needs asm/compat.h also to be included directly since this is included conditionally from include/compat.h. This does seem to be typical of arm64 as I was not completely able to get rid of asm/compat.h includes for arm64 in this series. My plan is to have separate patches to get rid of asm/compat.h includes for the architectures that are not straight forward to keep this series simple. I will fix this and update the series. I ran across the same thing in two more files during randconfig testing on arm64 now, adding this fixup on top for the moment, but maybe there is a better way: I was looking at how Al tested his uaccess patches: https://www.spinics.net/lists/linux-fsdevel/msg108752.html He seems to be running the kbuild bot tests on his own git. Is it possible to verify it this way on the 2038 tree? Or, I could host a tree also. The kbuild bot should generally pick up any branch on git.kernel.org, and the patches sent to the mailing list. It tests a lot of things configurations, but I tend to find some things that it doesn't find by doing lots of randconfig builds on fewer target architectures (I only build arm, arm64 and x86 regularly). I remember that there was some discussion about a method to get the bot to test other branches (besides asking Fengguang to add it manually), but I don't remember what came out of that. People can send email to me or l...@intel.com for adding new git URLs to 0day tests. Such requests are very welcome. Server load is not a problem -- don't worry about your git pushes adding our test load. By default all branches in a git tree will be tested, unless there are explicit blacklist/whitelist. We also have scripts to scan git.kernel.org/github/LKML looking for possible new git URLs to add to 0day kbuild tests. However depending on the team's maintenance pressure they may or may not run frequently. Thanks, Fengguang
Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs
On 3/16/18 6:04 PM, Steve Wise wrote: Anybody understand why the PPC implementation of writeX_relaxed() isn't relaxed? You probably should ask that on the linuxppc-dev@lists.ozlabs.org mailing list. I've always wondered why PowerPC has non-standard I/O accessors. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH] powerpc: wii.dts: Add drive slot LED
The Wii has a blue LED in the disk drive slot, which is controlled via a
GPIO line. Add this LED to wii.dts, and mark it as a panic-indicator.
Signed-off-by: Jonathan Neuschäfer
---
arch/powerpc/boot/dts/wii.dts | 12
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/boot/dts/wii.dts b/arch/powerpc/boot/dts/wii.dts
index d697f706fc22..a946a8f4bee3 100644
--- a/arch/powerpc/boot/dts/wii.dts
+++ b/arch/powerpc/boot/dts/wii.dts
@@ -13,6 +13,7 @@
*/
/dts-v1/;
+#include
/*
* This is commented-out for now.
@@ -228,5 +229,16 @@
interrupts = <2>;
};
};
+
+ gpio-leds {
+ compatible = "gpio-leds";
+
+ /* This is the blue LED in the disk drive slot */
+ drive-slot {
+ label = "wii:blue:drive_slot";
+ gpios = <&GPIO 5 GPIO_ACTIVE_HIGH>;
+ panic-indicator;
+ };
+ };
};
--
2.16.2
Re: [PATCH v3 18/18] infiniband: cxgb4: Eliminate duplicate barriers on weakly-ordered archs
+linuxppc-dev@lists.ozlabs.org
On 3/17/2018 11:05 AM, Jason Gunthorpe wrote:
> On Sat, Mar 17, 2018 at 12:25:14AM -0400, Sinan Kaya wrote:
>> On 3/17/2018 12:03 AM, Sinan Kaya wrote:
>>> On 3/16/2018 11:40 PM, Sinan Kaya wrote:
I'll change writel_relaxed() with __raw_writel() in the series like you
suggested
and also look at your other comments.
>>>
>>> I spoke too soon.
>>>
>>> Now that I realized, code needs to follow one of the following patterns for
>>> correctness
>>>
>>> 1)
>>> wmb()
>>> writel()/writel_relaxed()
>>>
>>> or
>>>
>>> 2)
>>> wmb()
>>> __raw_wrltel()
>>> mmiowb()
>>>
>>> but definitely not
>>>
>>> wmb()
>>> __raw_wrltel()
>>>
>>> Since #1 == #2, I'll stick to my current implementation of writel_relaxed()
>>>
>>> Changing writel() to writel_relaxed() or __raw_writel() isn't enough.
>>> PowerPC needs mmiowb()
>>> for correctness. ARM's mmiowb() implementation is empty.
>>>
>>> So, there is no one size fits all solution with the current state of
>>> affairs.
>>>
>>>
>>
>> I think I finally got what you mean.
>>
>> Code seems to have
>>
>> wmb()
>> writel()/writeq()
>> wmb()
>>
>> this can be safely replaced with
>>
>> wmb()
>> __raw_writel()/__raw_writeq()
>> wmb()
>>
>> This will work on all arches. Below is the new version. Let me know if this
>> is OK.
>>
>> +++ b/drivers/infiniband/hw/cxgb4/t4.h
>> @@ -457,7 +457,7 @@ static inline void pio_copy(u64 __iomem *dst, u64 *src)
>> int count = 8;
>>
>> while (count) {
>> - writeq(*src, dst);
>> + __raw_writeq(*src, dst);
>> src++;
>> dst++;
>> count--;
>> @@ -477,15 +477,16 @@ static inline void t4_ring_sq_db(struct t4_wq *wq, u16
>> inc, union t4_wr *wqe)
>> (u64 *)wqe);
>> } else {
>> pr_debug("DB wq->sq.pidx = %d\n", wq->sq.pidx);
>> - writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
>> - wq->sq.bar2_va + SGE_UDB_KDOORBELL);
>> + __raw_writel(PIDX_T5_V(inc) | QID_V(wq->sq.bar2_qid),
>> +wq->sq.bar2_va + SGE_UDB_KDOORBELL);
>> }
>>
>> /* Flush user doorbell area writes. */
>> wmb();
>> return;
>> }
>> - writel(QID_V(wq->sq.qid) | PIDX_V(inc), wq->db);
>> + __raw_writel(QID_V(wq->sq.qid) | PIDX_V(inc), wq->db);
>> + mmiowmb()
>> }
>>
>> static inline void t4_ring_rq_db(struct t4_wq *wq, u16 inc,
>> @@ -502,15 +503,16 @@ static inline void t4_ring_rq_db(struct t4_wq *wq, u16
>> inc,
>> (void *)wqe);
>> } else {
>> pr_debug("DB wq->rq.pidx = %d\n", wq->rq.pidx);
>> - writel(PIDX_T5_V(inc) | QID_V(wq->rq.bar2_qid),
>> - wq->rq.bar2_va + SGE_UDB_KDOORBELL);
>> + __raw_writel(PIDX_T5_V(inc) | QID_V(wq->rq.bar2_qid),
>> +wq->rq.bar2_va + SGE_UDB_KDOORBELL);
>> }
>>
>> /* Flush user doorbell area writes. */
>> wmb();
>> return;
>> }
>> - writel(QID_V(wq->rq.qid) | PIDX_V(inc), wq->db);
>> + __raw_writel(QID_V(wq->rq.qid) | PIDX_V(inc), wq->db);
>> + mmiowmb();
>
> No! NAK! Adding *new* barriers to use the *wrong* accessor is crazy!
>
> Your first patch was right, replacing
> wmb()
> writel()
>
> With wmb(); writel_relaxed() is fine, and helps ARM.
>
> The problem with PPC has nothing to do with the writel, it is with the
> spinlock, and we can't improve it without adding some new
> infrastructure. Certainly don't hack around the problem by using
> __raw_writel in multi-arch drivers!
>
> In userspace we settled on something like this as a pattern:
>
> mmio_wc_spin_lock()
> writel_wc_mmio()
> mmio_wc_spin_unlock()
>
> Where mmio_wc_spin_unlock incorporates the mmiowmb and is defined to
> fully contain all MMIO access to WC and UC memory.
>
> Due to our userspace the pattern is specifically used with MMIO writes
> to WC BAR memory, but it could be generalized..
>
> This allows all known arches to use the best code in all cases. x86
> can even optimize a lot and combine the mmiowmb() into the
> spin_unlock, apparently.
Given both Dave [1] and Jason's feedback, we have to ask PowerPC developers
to fix writel_relaxed().
Otherwise, PowerPC won't be able to take advantage of the optimizations
introduced in this series.
I don't think we need writel_really_relaxed() API.
Somebody also has to take a task and work very hard to get rid of __raw_writeX()
APIs in drivers/net directory. It looked like a very common practice though
it clearly violates multiarch portability concerns Jason and Deve highlighted.
This will be the next version:
iff --git a/drivers/infiniband/hw/cxgb4/t4.h b/drivers/
Re: [PATCH 01/19] xmon: Use __printf markup to silence compiler
Hi Mathieu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mathieu-Malaterre/Start-using-__printf-attribute-single-commit-series/20180318-035038
config: powerpc64-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc64
Note: the
linux-review/Mathieu-Malaterre/Start-using-__printf-attribute-single-commit-series/20180318-035038
HEAD 070c2d653f1924feb0363271515fea85920b80f9 builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
arch/powerpc/xmon/xmon.c: In function 'cpu_cmd':
>> arch/powerpc/xmon/xmon.c:1168:18: error: format '%x' expects argument of
>> type 'unsigned int', but argument 2 has type 'long unsigned int'
>> [-Werror=format=]
printf("cpu 0x%x isn't in xmon\n", cpu);
~^
%lx
arch/powerpc/xmon/xmon.c:1182:19: error: format '%x' expects argument of
type 'unsigned int', but argument 2 has type 'long unsigned int'
[-Werror=format=]
printf("cpu 0x%x didn't take control\n", cpu);
~^
%lx
arch/powerpc/xmon/xmon.c: In function 'bpt_cmds':
arch/powerpc/xmon/xmon.c:1392:15: error: format '%x' expects argument of
type 'unsigned int', but argument 2 has type 'long int' [-Werror=format=]
printf("%2x %s ", BP_NUM(bp),
~~^
%2lx
arch/powerpc/xmon/xmon.c: In function 'excprint':
arch/powerpc/xmon/xmon.c:1607:31: error: format '%lx' expects argument of
type 'long unsigned int', but argument 4 has type 'struct pt_regs *'
[-Werror=format=]
printf("Vector: %lx %s at [%lx]\n", fp->trap, getvecname(trap), fp);
~~^
arch/powerpc/xmon/xmon.c:1611:9: error: too many arguments for format
[-Werror=format-extra-args]
printf("lr: ", fp->link);
^~
arch/powerpc/xmon/xmon.c:1623:26: error: format '%lx' expects argument of
type 'long unsigned int', but argument 2 has type 'struct task_struct *'
[-Werror=format=]
printf(" current = 0x%lx\n", current);
~~^
>> arch/powerpc/xmon/xmon.c:1625:26: error: format '%lx' expects argument of
>> type 'long unsigned int', but argument 2 has type 'struct paca_struct *'
>> [-Werror=format=]
printf(" paca= 0x%lx\t softe: %d\t irq_happened: 0x%02x\n",
~~^
arch/powerpc/xmon/xmon.c:1629:25: error: format '%ld' expects argument of
type 'long int', but argument 2 has type 'pid_t {aka int}' [-Werror=format=]
printf("pid = %ld, comm = %s\n",
~~^
%d
arch/powerpc/xmon/xmon.c: In function 'prregs':
>> arch/powerpc/xmon/xmon.c:1665:17: error: format '%ld' expects argument of
>> type 'long int', but argument 2 has type 'int' [-Werror=format=]
printf("R%.2ld = "REG" R%.2ld = "REG"\n",
^
%.2d
arch/powerpc/xmon/xmon.c:1665:11: error: format '%ld' expects argument of
type 'long int', but argument 4 has type 'int' [-Werror=format=]
printf("R%.2ld = "REG" R%.2ld = "REG"\n",
^~~
n, fp->gpr[n], n+16, fp->gpr[n+16]);
arch/powerpc/xmon/xmon.c:1665:34: note: format string is defined here
printf("R%.2ld = "REG" R%.2ld = "REG"\n",
^
%.2d
arch/powerpc/xmon/xmon.c:1669:17: error: format '%ld' expects argument of
type 'long int', but argument 2 has type 'int' [-Werror=format=]
printf("R%.2ld = "REG" R%.2ld = "REG"\n",
^
%.2d
arch/powerpc/xmon/xmon.c:1669:11: error: format '%ld' expects argument of
type 'long int', but argument 4 has type 'int' [-Werror=format=]
printf("R%.2ld = "REG" R%.2ld = "REG"\n",
^~~
n, fp->gpr[n], n+7, fp->gpr[n+7]);
~~~
arch/powerpc/xmon/xmon.c:1669:34: note: format string is defined here
printf("R%.2ld = "REG" R%.2ld = "REG"\n",
^
%.2d
arch/powerpc/xmon/xmon.c: In function 'dump_206_sprs':
arch/powerpc/xmon/xmon.c:1778:54: error: format '%x' expects argument of
type 'unsigned int', but argument 4 has type 'long unsigned int'
[-Werror=format=]
printf("srr0 = %.16lx srr1 = %.16lx dsisr = %.8x\n",
~~~^
Re: [PATCH 01/19] xmon: Use __printf markup to silence compiler
Hi Mathieu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mathieu-Malaterre/Start-using-__printf-attribute-single-commit-series/20180318-035038
config: powerpc-currituck_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
arch/powerpc/xmon/xmon.c: In function 'xmon_core':
>> arch/powerpc/xmon/xmon.c:523:47: error: format '%lx' expects argument of
>> type 'long unsigned int', but argument 3 has type 'int' [-Werror=format=]
printf("cpu 0x%x stopped at breakpoint 0x%lx (",
~~^
%x
arch/powerpc/xmon/xmon.c: In function 'cpu_cmd':
arch/powerpc/xmon/xmon.c:1168:18: error: format '%x' expects argument of
type 'unsigned int', but argument 2 has type 'long unsigned int'
[-Werror=format=]
printf("cpu 0x%x isn't in xmon\n", cpu);
~^
%lx
arch/powerpc/xmon/xmon.c:1182:19: error: format '%x' expects argument of
type 'unsigned int', but argument 2 has type 'long unsigned int'
[-Werror=format=]
printf("cpu 0x%x didn't take control\n", cpu);
~^
%lx
arch/powerpc/xmon/xmon.c: In function 'bpt_cmds':
arch/powerpc/xmon/xmon.c:1365:32: error: format '%lx' expects argument of
type 'long unsigned int', but argument 2 has type 'int' [-Werror=format=]
printf("Cleared breakpoint %lx (", BP_NUM(bp));
~~^
%x
arch/powerpc/xmon/xmon.c: In function 'excprint':
arch/powerpc/xmon/xmon.c:1607:31: error: format '%lx' expects argument of
type 'long unsigned int', but argument 4 has type 'struct pt_regs *'
[-Werror=format=]
printf("Vector: %lx %s at [%lx]\n", fp->trap, getvecname(trap), fp);
~~^
arch/powerpc/xmon/xmon.c:1611:9: error: too many arguments for format
[-Werror=format-extra-args]
printf("lr: ", fp->link);
^~
arch/powerpc/xmon/xmon.c:1623:26: error: format '%lx' expects argument of
type 'long unsigned int', but argument 2 has type 'struct task_struct *'
[-Werror=format=]
printf(" current = 0x%lx\n", current);
~~^
arch/powerpc/xmon/xmon.c:1629:25: error: format '%ld' expects argument of
type 'long int', but argument 2 has type 'pid_t {aka int}' [-Werror=format=]
printf("pid = %ld, comm = %s\n",
~~^
%d
current->pid, current->comm);
arch/powerpc/xmon/xmon.c: In function 'prregs':
arch/powerpc/xmon/xmon.c:1674:22: error: format '%x' expects argument of
type 'unsigned int', but argument 3 has type 'long unsigned int'
[-Werror=format=]
printf("R%.2d = %.8x%s", n, fp->gpr[n],
~~~^~~
%.8lx
arch/powerpc/xmon/xmon.c: In function 'dump_by_size':
arch/powerpc/xmon/xmon.c:2567:16: error: format '%lx' expects argument of
type 'long unsigned int', but argument 3 has type 'u64 {aka long long unsigned
int}' [-Werror=format=]
printf("%0*lx", size * 2, val);
^
%0*llx
arch/powerpc/xmon/xmon.c: In function 'generic_inst_dump':
arch/powerpc/xmon/xmon.c:197:14: error: format '%x' expects argument of type
'unsigned int', but argument 3 has type 'long unsigned int' [-Werror=format=]
#define REG "%.8lx"
^
arch/powerpc/xmon/xmon.c:2731:11: note: in expansion of macro 'REG'
printf(REG" %.8x", adr, inst);
^~~
arch/powerpc/xmon/xmon.c:2731:20: note: format string is defined here
printf(REG" %.8x", adr, inst);
~~~^
%.8lx
arch/powerpc/xmon/xmon.c: In function 'memdiffs':
arch/powerpc/xmon/xmon.c:2863:17: error: format '%x' expects argument of
type 'unsigned int', but argument 2 has type 'unsigned char *' [-Werror=format=]
printf("%.16x %.2x # %.16x %.2x\n", p1 - 1,
^ ~~
%.16hhn
arch/powerpc/xmon/xmon.c:2863:30: error: format '%x' expects argument of
type 'unsigned int', but argument 4 has type 'unsigned char *' [-Werror=format=]
printf("%.16x %.2x # %.16x %.2x\n", p1 - 1,
^
%.16hhn
p1[-1], p2 - 1, p2[-1]);
~~
Re: [PATCH V4 1/3] powerpc/mm: Add support for handling > 512TB address in SLB miss
Michael Ellerman writes:
> Hi Aneesh,
>
> Some comments ...
>
> "Aneesh Kumar K.V" writes:
>> For address above 512TB we allocate additional mmu context. To make it all
>> easy address above 512TB is handled with IR/DR=1 and with stack frame setup.
>>
>> We do the additional context allocation in SLB miss handler. If the context
>> is
>> not allocated, we enable interrupts and allocate the context and retry the
>> access which will again result in a SLB miss.
>>
>> Signed-off-by: Aneesh Kumar K.V
>
> This doesn't build for g5_defconfig:
>
> In file included from ../arch/powerpc/include/asm/mmu.h:314:0,
>from ../arch/powerpc/include/asm/lppaca.h:36,
>from ../arch/powerpc/include/asm/paca.h:21,
>from ../arch/powerpc/include/asm/hw_irq.h:63,
>from ../arch/powerpc/include/asm/irqflags.h:12,
>from ../include/linux/irqflags.h:16,
>from ../include/linux/spinlock.h:54,
>from ../include/linux/mmzone.h:8,
>from ../arch/powerpc/include/asm/pgtable.h:7,
>from ../arch/powerpc/mm/slb.c:17:
> ../arch/powerpc/mm/slb.c: In function ‘handle_multi_context_slb_miss’:
> ../arch/powerpc/include/asm/book3s/64/mmu.h:208:25: error: array subscript
> is above array bounds [-Werror=array-bounds]
> return ctx->extended_id[index];
>^~~
> ../arch/powerpc/mm/slb.c:417:30: error: array subscript is above array
> bounds [-Werror=array-bounds]
> if (!mm->context.extended_id[index])
> ~~~^~~
> ../arch/powerpc/mm/slb.c:418:26: error: array subscript is above array
> bounds [-Werror=array-bounds]
> mm->context.extended_id[index] = context_id;
> ~~~^~~
I guess gcc is getting it wrong with ARRAY_SIZE is 1. I added the below:
static inline int get_ea_context(mm_context_t *ctx, unsigned long ea)
{
int index = ea >> MAX_EA_BITS_PER_CONTEXT;
if (likely(index < ARRAY_SIZE(ctx>extended_id)))
return ctx->extended_id[index];
/* should never happen */
BUG();
}
>
>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h
>> b/arch/powerpc/include/asm/book3s/64/hash-4k.h
>> index 67c5475311ee..af2ba9875f18 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
>> @@ -11,6 +11,12 @@
>> #define H_PUD_INDEX_SIZE 9
>> #define H_PGD_INDEX_SIZE 9
>>
>> +/*
>> + * No of address bits below which we use the default context
>
> Can we use "Number", "No" means no, to mean number you need "No." but
> that's ugly.
>
>> + * for slb allocation. For 4k this is 64TB.
>> + */
>> +#define H_BITS_FIRST_CONTEXT46
>
> It's actually the number of address bits *per* context right?
>
> "Context" is also a bit of a overloaded term, eg. context !=
> mm_context_t, maybe "context id" would be clearer?
>
> So maybe H_BITS_PER_CONTEXT_ID?
>
>
> This values is essentially VA_BITS - CONTEXT_BITS right?
>
> Except VA_BITS is actually 65 in this case, but that's not clear at all
> from the code :/
>
> It'd be really nice if this was calculated, or if the other values
> (VA_BITS/CONTEXT_BITS) were calculated based on it.
How about
/*
* Each context is 512TB. But on 4k we restrict our max TASK size to 64TB
* Hence also limit max EA bits to 64TB.
*/
#define MAX_EA_BITS_PER_CONTEXT 46
The VA bits is a bit complex with different platforms having different
restrictions. With 4k page size we still use VA bits as 68. We limit the
task size to 64TB there because of performance issue w.r.t to supporting
largere task size with 4K linux page size.
Now on p4 and p5 we do have hardware restrictions on VA bits. We default
there to 65.
IMHO we should keep the above #define independent of VA bits. It is an
indication of size of the address space context, which can possibily
smaller than what the EA and context bits combination can support as in
4K.
>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> b/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> index 3bcf269f8f55..0ee0fc1ad675 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
>> @@ -6,6 +6,11 @@
>> #define H_PMD_INDEX_SIZE 10
>> #define H_PUD_INDEX_SIZE 7
>> #define H_PGD_INDEX_SIZE 8
>
> Can we get some newlines between code?
>
>> +/*
>> + * No of address bits below which we use the default context
>> + * for slb allocation. For 64k this is 512TB.
>> + */
>> +#define H_BITS_FIRST_CONTEXT49
The coding style I was following here and other parts of code was
for multi block comments.
a = 10;
/*
* Comment here
*/
and for single line comment
a = 10;
/* Comment here */
The idea was the first line of the multiblock comment introduced the
blank line needed. I have switched to what you suggested. Let me know if
y
