Re: [PATCH] laptop-mode-2.6.0 version 5

From: Jens Axboe
Date: Fri Jan 02 2004 - 06:29:47 EST


On Fri, Jan 02 2004, Hugang wrote:
> Organization: Beijing Soul
> X-Mailer: Sylpheed version 0.9.8claws (GTK+ 1.2.10; powerpc-unknown-linux-gnu)
> Mime-Version: 1.0
> Content-Type: text/plain; charset=US-ASCII
> Content-Transfer-Encoding: 7bit
>
> On Thu, 1 Jan 2004 19:35:45 +0100
> Jens Axboe <axboe@xxxxxxx> wrote:
>
> > Patch is obviously bogus, just look at the comm definition in sched.h:
> >
> > char comm[16];
> >
> > IO submission must happen in process context, so we also know that
> > current is valid.
>
> You are right. But why add this patch, My laptop not crash when I
> enable block dump, So I try to find where is the Bug. Final, The bug

I dunno, I can't possibly tell since you haven't given any info about
this crash. Where does it crash, do you have an oops? All I could say
from your report + patch is that it wasn't valid. There's just no way
for current->comm to be NULL, so your patch couldn't possibly have made
a difference.

> is in sector_t, I was enable CONFIG_LBD, So sector_t is u64, So We
> have to change the code when enable CONFIG_LBD.
>
> I'd like the 2.4 style so add count number into printf.
>
> Here is the patch fix it
> +
> + if (unlikely(block_dump)) {
> + char b[BDEVNAME_SIZE];
> + printk("%s(%d): %s block %llu/%u on %s\n",
> + current->comm, current->pid,
> + (rw & WRITE) ? "WRITE" : (rw == READA ? "READA" : "READ"),
> + (u64)bio->bi_sector, count, bdevname(bio->bi_bdev,b));
> + }

It's best to keep the line as minimal as possible, count isn't really
very interesting. What is interesting is process, offset (for finding
the file, if you need to), and data direction.

> I think, also have this bug in 2.4.23, here is the patch for it, Hope can helpful.
> Index: linux-2.4.23/drivers/block/ll_rw_blk.c
> ===================================================================
> --- linux-2.4.23/drivers/block/ll_rw_blk.c (revision 4)
> +++ linux-2.4.23/drivers/block/ll_rw_blk.c (working copy)
> @@ -1298,7 +1298,7 @@
> wake_up(&bh->b_wait);
>
> if (block_dump)
> - printk(KERN_DEBUG "%s: %s block %lu/%u on %s\n", current->comm, rw == WRITE ? "WRITE" : "READ", bh->b_rsector, count, kdevname(bh->b_rdev));
> + printk(KERN_DEBUG "%s: %s block %llu/%u on %s\n", current->comm, rw == WRITE ? "WRITE" : "READ", (u64)bh->b_rsector, count, kdevname(bh->b_rdev));

2.4 stock doesn't have 64-bit sectors, please consult (again) the
canonical source (include file). There's no need to cast.

--
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/