How best to get the size of a blockdev from a file?

How best to get the size of a blockdev from a file?

[David Howells]

Hi Christoph,

It seems that my use of i_size_read(file_inode(in)) in filemap_splice_read()
to get the size of the file to be spliced from doesn't work in the case of
blockdevs and it always returns 0.

What would be the best way to get the blockdev size?  Look at
file->f_mapping->i_size maybe?

David

Re: How best to get the size of a blockdev from a file?

[Kanchan Joshi]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]

On Tue, Apr 18, 2023 at 11:00:12AM +0100, David Howells wrote:
>Hi Christoph,
>
>It seems that my use of i_size_read(file_inode(in)) in filemap_splice_read()
>to get the size of the file to be spliced from doesn't work in the case of
>blockdevs and it always returns 0.
>
>What would be the best way to get the blockdev size?  Look at
>file->f_mapping->i_size maybe?

bdev_nr_bytes(I_BDEV(file->f_mapping->host))
should work I suppose.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: How best to get the size of a blockdev from a file?

[Jason Yan]

On 2023/4/18 20:28, Kanchan Joshi wrote:
> On Tue, Apr 18, 2023 at 11:00:12AM +0100, David Howells wrote:
>> Hi Christoph,
>>
>> It seems that my use of i_size_read(file_inode(in)) in 
>> filemap_splice_read()
>> to get the size of the file to be spliced from doesn't work in the 
>> case of
>> blockdevs and it always returns 0.
>>
>> What would be the best way to get the blockdev size?  Look at
>> file->f_mapping->i_size maybe?
> 
> bdev_nr_bytes(I_BDEV(file->f_mapping->host))
> should work I suppose.
> 

This needs the caller to check if the file is a blockdev. Can we fill 
the upper inode size which belongs to devtmpfs so that the generic code 
do not need to aware the low level inode type?

Re: How best to get the size of a blockdev from a file?

[Al Viro]

On Tue, Apr 18, 2023 at 10:05:12PM +0800, Jason Yan wrote:
> On 2023/4/18 20:28, Kanchan Joshi wrote:
> > On Tue, Apr 18, 2023 at 11:00:12AM +0100, David Howells wrote:
> > > Hi Christoph,
> > > 
> > > It seems that my use of i_size_read(file_inode(in)) in
> > > filemap_splice_read()
> > > to get the size of the file to be spliced from doesn't work in the
> > > case of
> > > blockdevs and it always returns 0.
> > > 
> > > What would be the best way to get the blockdev size?  Look at
> > > file->f_mapping->i_size maybe?
> > 
> > bdev_nr_bytes(I_BDEV(file->f_mapping->host))
> > should work I suppose.
> > 
> 
> This needs the caller to check if the file is a blockdev. Can we fill the
> upper inode size which belongs to devtmpfs so that the generic code do not
> need to aware the low level inode type?

We do:

static inline loff_t bdev_nr_bytes(struct block_device *bdev)
{
	return (loff_t)bdev_nr_sectors(bdev) << SECTOR_SHIFT;
}

static inline sector_t bdev_nr_sectors(struct block_device *bdev)
{
        return bdev->bd_nr_sectors;
}

$ git grep -n -w bd_nr_sectors
block/genhd.c:64:       bdev->bd_nr_sectors = sectors;
block/partitions/core.c:92:     bdev->bd_nr_sectors = sectors;
include/linux/blk_types.h:42:   sector_t                bd_nr_sectors;
include/linux/blkdev.h:785:     return bdev->bd_nr_sectors;

and if you look into the functions with those assignments you'll see
void set_capacity(struct gendisk *disk, sector_t sectors)
{
        struct block_device *bdev = disk->part0;

	spin_lock(&bdev->bd_size_lock);
	i_size_write(bdev->bd_inode, (loff_t)sectors << SECTOR_SHIFT);
	bdev->bd_nr_sectors = sectors;
	spin_unlock(&bdev->bd_size_lock);
}

and

static void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors)
{
        spin_lock(&bdev->bd_size_lock);
	i_size_write(bdev->bd_inode, (loff_t)sectors << SECTOR_SHIFT);
	bdev->bd_nr_sectors = sectors;
	spin_unlock(&bdev->bd_size_lock);
}

As you can see, both do i_size_write() on bdev->bd_inode with the value
equal to what bdev_nr_bytes() will return after the store to ->bd_nr_sectors.

Now, bdev->bd_inode always points to inode coallocated with bdev
(see bdev_alloc() for details) and that's what we have
->f_mapping->host pointing to for opened file after
blkdev_open() - see
        filp->f_mapping = bdev->bd_inode->i_mapping;
in there combined with inode->i_mapping.host set to inode by
inode_init_always()<-alloc_inode()<-new_inode_pseudo()<-new_inode()<-bdev_alloc().

IOW, i_size_read(file->f_mapping->host) is correct answer for any kind of file.

Re: How best to get the size of a blockdev from a file?

[Jason Yan]

On 2023/4/21 3:04, Al Viro wrote:
> On Tue, Apr 18, 2023 at 10:05:12PM +0800, Jason Yan wrote:
>> On 2023/4/18 20:28, Kanchan Joshi wrote:
>>> On Tue, Apr 18, 2023 at 11:00:12AM +0100, David Howells wrote:
>>>> Hi Christoph,
>>>>
>>>> It seems that my use of i_size_read(file_inode(in)) in
>>>> filemap_splice_read()
>>>> to get the size of the file to be spliced from doesn't work in the
>>>> case of
>>>> blockdevs and it always returns 0.
>>>>
>>>> What would be the best way to get the blockdev size?  Look at
>>>> file->f_mapping->i_size maybe?
>>>
>>> bdev_nr_bytes(I_BDEV(file->f_mapping->host))
>>> should work I suppose.
>>>
>>
>> This needs the caller to check if the file is a blockdev. Can we fill the
>> upper inode size which belongs to devtmpfs so that the generic code do not
>> need to aware the low level inode type?
> 
> We do:
> 
> static inline loff_t bdev_nr_bytes(struct block_device *bdev)
> {
> 	return (loff_t)bdev_nr_sectors(bdev) << SECTOR_SHIFT;
> }
> 
> static inline sector_t bdev_nr_sectors(struct block_device *bdev)
> {
>          return bdev->bd_nr_sectors;
> }
> 
> $ git grep -n -w bd_nr_sectors
> block/genhd.c:64:       bdev->bd_nr_sectors = sectors;
> block/partitions/core.c:92:     bdev->bd_nr_sectors = sectors;
> include/linux/blk_types.h:42:   sector_t                bd_nr_sectors;
> include/linux/blkdev.h:785:     return bdev->bd_nr_sectors;
> 
> and if you look into the functions with those assignments you'll see
> void set_capacity(struct gendisk *disk, sector_t sectors)
> {
>          struct block_device *bdev = disk->part0;
> 
> 	spin_lock(&bdev->bd_size_lock);
> 	i_size_write(bdev->bd_inode, (loff_t)sectors << SECTOR_SHIFT);
> 	bdev->bd_nr_sectors = sectors;
> 	spin_unlock(&bdev->bd_size_lock);
> }
> 
> and
> 
> static void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors)
> {
>          spin_lock(&bdev->bd_size_lock);
> 	i_size_write(bdev->bd_inode, (loff_t)sectors << SECTOR_SHIFT);
> 	bdev->bd_nr_sectors = sectors;
> 	spin_unlock(&bdev->bd_size_lock);
> }
> 
> As you can see, both do i_size_write() on bdev->bd_inode with the value
> equal to what bdev_nr_bytes() will return after the store to ->bd_nr_sectors.
> 
> Now, bdev->bd_inode always points to inode coallocated with bdev
> (see bdev_alloc() for details) and that's what we have
> ->f_mapping->host pointing to for opened file after
> blkdev_open() - see
>          filp->f_mapping = bdev->bd_inode->i_mapping;
> in there combined with inode->i_mapping.host set to inode by
> inode_init_always()<-alloc_inode()<-new_inode_pseudo()<-new_inode()<-bdev_alloc().
> 
> IOW, i_size_read(file->f_mapping->host) is correct answer for any kind of file.

Ah, yes. Thanks for your patient explanation.

Jason

Re: How best to get the size of a blockdev from a file?

[Christoph Hellwig]

On Tue, Apr 18, 2023 at 11:00:12AM +0100, David Howells wrote:
> Hi Christoph,
> 
> It seems that my use of i_size_read(file_inode(in)) in filemap_splice_read()
> to get the size of the file to be spliced from doesn't work in the case of
> blockdevs and it always returns 0.
>
> What would be the best way to get the blockdev size?  Look at
> file->f_mapping->i_size maybe?

Yes.  Everything using an inode in generic read/write helpers always
needs to use file->f_mapping->host to get at the inode.  Not just for
the size.