Reading page from given block device

Reading page from given block device

[Pavel Machek]

Hi!

For swsusp, I kind of need to read 4K from given block device.

Here's my attempt:

static int bdev_read_page(kdev_t dev, long pos, void *buf)
{
        struct buffer_head *bh;
        struct block_device *bdev;

        if (pos%PAGE_SIZE) panic("Sorry, dave, I can't let you do
that!\n");
        bdev = bdget(kdev_t_to_nr(dev));
        if (!bdev) {
                printk("No block device for %s\n", __bdevname(dev));
                BUG();
        }
        printk("C");
        bh = __bread(bdev, pos/PAGE_SIZE, PAGE_SIZE);
        printk("D");
        bdput(bdev);
        if (!bh || (!bh->b_data)) {
                return -1;
        }
        memcpy(buf, bh->b_data, PAGE_SIZE);
        bforget(bh);                    /* FIXME: maybe bforget should
be here */
        return 0;
}

It works *once*, second time it deadlocks in __bread(). I tried both
bforget() and brelse(). Kernel is 2.5.13. What am I doing wrong/what's
wrong?

									Pavel
-- 
(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

Re: Reading page from given block device

[Andrew Morton]

Pavel Machek wrote:
> 
> Hi!
> 
> For swsusp, I kind of need to read 4K from given block device.
> 
> Here's my attempt:
> 
> static int bdev_read_page(kdev_t dev, long pos, void *buf)
> {
>         struct buffer_head *bh;
>         struct block_device *bdev;
> 
>         if (pos%PAGE_SIZE) panic("Sorry, dave, I can't let you do
> that!\n");

It's possible I guess that someone has a pinned buffer against
the same page which has a different block size.  See the "lock up"
comment over __getblk().

>         bdev = bdget(kdev_t_to_nr(dev));
>         if (!bdev) {
>                 printk("No block device for %s\n", __bdevname(dev));
>                 BUG();
>         }
>         printk("C");
>         bh = __bread(bdev, pos/PAGE_SIZE, PAGE_SIZE);
>         printk("D");
>         bdput(bdev);
>         if (!bh || (!bh->b_data)) {
>                 return -1;
>         }
>         memcpy(buf, bh->b_data, PAGE_SIZE);

You'll need to kmap bh->b_page before copying the data.

> 
> It works *once*, second time it deadlocks in __bread(). I tried both
> bforget() and brelse(). Kernel is 2.5.13. What am I doing wrong/what's
> wrong?

brelse is safer.

Please try 2.5.14.  2.5.13 had a few leaky problems which
could perhaps result in a pinned buffer which will cause
try_to_free_buffers() to fail, which triggers the __getblk()
nastiness.

Generally, if you're reading from a swap partition then
it may be better to use brw_page().  bread() is backed
by bdev->bd_inode->i_mapping, and there may be coherency
problems if the target blocks are currently in swapper_space. 
Although probably your bdget() here will create a new inode
with no pagecache, so it'll work OK.

It really depends on what you're trying to do.  It may
be best to just cook up a local buffer_head and submit
the darn thing.

-

Re: Reading page from given block device

[Andrew Morton]

Andrew Morton wrote:
> 
> ..
> >         memcpy(buf, bh->b_data, PAGE_SIZE);
> 
> You'll need to kmap bh->b_page before copying the data.

No you won't.  blockdev mappings don't use highmem.

-

Re: Reading page from given block device

[Pavel Machek]

Hi!

> > For swsusp, I kind of need to read 4K from given block device.
> > 
> > Here's my attempt:
> > 
> > static int bdev_read_page(kdev_t dev, long pos, void *buf)
> > {
> >         struct buffer_head *bh;
> >         struct block_device *bdev;
> > 
> >         if (pos%PAGE_SIZE) panic("Sorry, dave, I can't let you do
> > that!\n");
> 
> It's possible I guess that someone has a pinned buffer against
> the same page which has a different block size.  See the "lock up"
> comment over __getblk().

Well, I'm doing it during boot, and this is swap partition; it should
not have been accessed previously.

> >         bdev = bdget(kdev_t_to_nr(dev));
> >         if (!bdev) {
> >                 printk("No block device for %s\n", __bdevname(dev));
> >                 BUG();
> >         }
> >         printk("C");
> >         bh = __bread(bdev, pos/PAGE_SIZE, PAGE_SIZE);
> >         printk("D");
> >         bdput(bdev);
> >         if (!bh || (!bh->b_data)) {
> >                 return -1;
> >         }
> >         memcpy(buf, bh->b_data, PAGE_SIZE);
> 
> You'll need to kmap bh->b_page before copying the data.

This machine does not have himem.

> > It works *once*, second time it deadlocks in __bread(). I tried both
> > bforget() and brelse(). Kernel is 2.5.13. What am I doing wrong/what's
> > wrong?
> 
> brelse is safer.
> 
> Please try 2.5.14.  2.5.13 had a few leaky problems which
> could perhaps result in a pinned buffer which will cause
> try_to_free_buffers() to fail, which triggers the __getblk()
> nastiness.
> 
> Generally, if you're reading from a swap partition then
> it may be better to use brw_page().  bread() is backed

It is swap partition, but system does not yet know its swap at that
point. This is next boot, that partition was not yet accessed.
								Pavel
-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: Reading page from given block device

[Pavel Machek]

Hi!
> > It is swap partition, but system does not yet know its swap at that
> > point. This is next boot, that partition was not yet accessed.
> >                                                                 Pavel
> 
> In that case, I don't know.  Sorry.  sysrq-T is your
> friend (and kgdb is your god).  Let me know...

Call trace is 

c01229ed -- bdev_read_page
c0134b5b -- __bread

That's stable (as expected). Under that, it was seen in

c0134a57 -- __getblk
	    blk_get_queue
	    ata_get_queue

Second try:

c0134a28 -- __getblk
	    __get_hash_table

Third try:

c0134a28 -- __getblk
c01343cc -- __get_hash_table

Fourth try:

c0134abd -- __getblk

... I should get some sleep I guess...
								Pavel
-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: Reading page from given block device

[Andrew Morton]

Pavel Machek wrote:
> 
> ...
> Second try:
> 
> c0134a28 -- __getblk
>             __get_hash_table

hmm.  It looks like your blockdev may have the wrong
blocksize in bd_inode->i_blkbits, and __get_hash_table
cannot find the 4k block against the 1k blocksize device.
It keeps returning zero.

The new pagecache-based __get_hash_table uses bd_inode->i_blkbits
to go from a block number to a pagecache index.  That works
fine when called via the official `bread()' function, but 
probably your __bread() approach has confused it.

Try running set_blocksize(bdev, PAGE_SIZE) before calling
__bread().  


-

Re: Reading page from given block device

[Alexander Viro]

On Thu, 9 May 2002, Pavel Machek wrote:

> Well, I'm doing it during boot, and this is swap partition; it should
> not have been accessed previously.
> 
> > >         bdev = bdget(kdev_t_to_nr(dev));
> > >         if (!bdev) {
> > >                 printk("No block device for %s\n", __bdevname(dev));
> > >                 BUG();
> > >         }
> > >         printk("C");

blkdev_open(bdev, FMODE_READ, O_RDONLY, BDEV_RAW)

> > >         bh = __bread(bdev, pos/PAGE_SIZE, PAGE_SIZE);
> > >         printk("D");
> > >         bdput(bdev);

nope - blkdev_put(bdev), and do it _after_ you are done with buffers.
Oh, and you need to do brelse().

> > >         if (!bh || (!bh->b_data)) {
> > >                 return -1;

However, I would really suggest to open the bugger once, do all IO and
then close it.  See how raw.c and friends deal with these problems.

Re: Reading page from given block device

[Pavel Machek]

Hi!

> > Well, I'm doing it during boot, and this is swap partition; it should
> > not have been accessed previously.
> > 
> > > >         bdev = bdget(kdev_t_to_nr(dev));
> > > >         if (!bdev) {
> > > >                 printk("No block device for %s\n", __bdevname(dev));
> > > >                 BUG();
> > > >         }
> > > >         printk("C");
> 
> blkdev_open(bdev, FMODE_READ, O_RDONLY, BDEV_RAW)

blkdev_open is 

fs.h:extern int blkdev_open(struct inode *, struct file *);

... I can't see how to use it in this context.

> > > >         if (!bh || (!bh->b_data)) {
> > > >                 return -1;
> 
> However, I would really suggest to open the bugger once, do all IO and
> then close it.  See how raw.c and friends deal with these problems.

Performance should not matter here.
								Pavel

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: Reading page from given block device

[Alexander Viro]

On Thu, 9 May 2002, Pavel Machek wrote:

> Hi!
> 
> > > Well, I'm doing it during boot, and this is swap partition; it should
> > > not have been accessed previously.
> > > 
> > > > >         bdev = bdget(kdev_t_to_nr(dev));
> > > > >         if (!bdev) {
> > > > >                 printk("No block device for %s\n", __bdevname(dev));
> > > > >                 BUG();
> > > > >         }
> > > > >         printk("C");
> > 
> > blkdev_open(bdev, FMODE_READ, O_RDONLY, BDEV_RAW)
> 
> blkdev_open is 

grr...  s/open/get/ - sorry.

> fs.h:extern int blkdev_open(struct inode *, struct file *);
> 
> ... I can't see how to use it in this context.
> 
> > > > >         if (!bh || (!bh->b_data)) {
> > > > >                 return -1;
> > 
> > However, I would really suggest to open the bugger once, do all IO and
> > then close it.  See how raw.c and friends deal with these problems.
> 
> Performance should not matter here.

Yeah, but amount of places that mess with kdev_t should.

Re: Reading page from given block device

[Pavel Machek]

Hi!

> > Hi!
> > 
> > For swsusp, I kind of need to read 4K from given block device.
> > 
> > Here's my attempt:
> > 
> > static int bdev_read_page(kdev_t dev, long pos, void *buf)
> > {
> >         struct buffer_head *bh;
> >         struct block_device *bdev;
> > 
> >         if (pos%PAGE_SIZE) panic("Sorry, dave, I can't let you do
> > that!\n");
> 
> It's possible I guess that someone has a pinned buffer against
> the same page which has a different block size.  See the "lock up"
> comment over __getblk().
> 
> >         bdev = bdget(kdev_t_to_nr(dev));
> >         if (!bdev) {
> >                 printk("No block device for %s\n", __bdevname(dev));
> >                 BUG();
> >         }
> >         printk("C");
> >         bh = __bread(bdev, pos/PAGE_SIZE, PAGE_SIZE);
> >         printk("D");
> >         bdput(bdev);
> >         if (!bh || (!bh->b_data)) {
> >                 return -1;
> >         }
> >         memcpy(buf, bh->b_data, PAGE_SIZE);
> 
> You'll need to kmap bh->b_page before copying the data.
> 
> > 
> > It works *once*, second time it deadlocks in __bread(). I tried both
> > bforget() and brelse(). Kernel is 2.5.13. What am I doing wrong/what's
> > wrong?
> 
> brelse is safer.
> 
> Please try 2.5.14.  2.5.13 had a few leaky problems which
> could perhaps result in a pinned buffer which will cause
> try_to_free_buffers() to fail, which triggers the __getblk()
> nastiness.

Went with brelse(), and switched to 2.5.14; still deadlocks on second
read.
								Pavel
-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.