On Mon, 13 Jan 2014, Mike Snitzer wrote:
> On Mon, Jan 13 2014 at 6:45pm -0500,
> Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
> >
> >
> > On Mon, 13 Jan 2014, Mike Snitzer wrote:
> >
> > > On Mon, Jan 13 2014 at 5:00pm -0500,
> > > Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
> > >
> > > > No.
> > > >
> > > > This changed patch inefficiently loops in bufio_prefetch_chunks for each
> > > > buffer that is read.
> > >
> > > Yeah, I overlooked the importance of preserving prefetch_area (which
> > > explains the unlikely you had). But my intent was to make your code
> > > less "special"; your if (DM_PREFETCH_CHUNKS) do { } while() code to
> > > avoid the extra indentation is pretty ugly.
> > >
> > > Given your current code, DM_PREFETCH_CHUNKS is always 12 so why not just
> > > remove the check?
> >
> > If someone ever changes it to a variable, the condition is there avoid
> > unneeded call to dm_bufio_prefetch. The compiler optimizes out the
> > constant expression, so it doesn't matter that it's there.
>
> How about move the conditional inside the do { } ?
>
> I really dislike the style you've used on this, is there anywhere else
> in the kernel that does this?
grep -r 'if (.*) do {' *
shows
arch/hexagon/include/asm/cmpxchg.h: if (size != 4) do { asm volatile("brkpt;\n"); } while (1);
drivers/media/usb/tm6000/tm6000-i2c.c:#define i2c_dprintk(lvl, fmt, args...) if (i2c_debug >= lvl) do { \
drivers/net/wireless/airo.c: if (rc == SUCCESS) do {
drivers/net/wireless/airo.c: if (rc == SUCCESS) do {
include/linux/zutil.h: if (k != 0) do {
lib/lzo/lzo1x_compress.c: if (t > 0) do {
lib/lzo/lzo1x_compress.c: if (t >= 16) do {
lib/lzo/lzo1x_compress.c: if (t > 0) do {
lib/zlib_deflate/deftree.c: if (s->last_lit != 0) do {
I don't think it's that problematic.
Mikulas
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
