Linux-Block Archive mirror
 help / color / mirror / Atom feed
* How best to get the size of a blockdev from a file?
@ 2023-04-18 10:00 ` David Howells
  2023-04-18 12:28   ` Kanchan Joshi
  2023-04-18 15:34   ` Christoph Hellwig
  0 siblings, 2 replies; 6+ messages in thread
From: David Howells @ 2023-04-18 10:00 UTC (permalink / raw
  To: hch; +Cc: dhowells, linux-block

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: How best to get the size of a blockdev from a file?
  2023-04-18 10:00 ` How best to get the size of a blockdev from a file? David Howells
@ 2023-04-18 12:28   ` Kanchan Joshi
  2023-04-18 14:05     ` Jason Yan
  2023-04-18 15:34   ` Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Kanchan Joshi @ 2023-04-18 12:28 UTC (permalink / raw
  To: David Howells; +Cc: hch, linux-block

[-- 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 --]



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: How best to get the size of a blockdev from a file?
  2023-04-18 12:28   ` Kanchan Joshi
@ 2023-04-18 14:05     ` Jason Yan
  2023-04-20 19:04       ` Al Viro
  0 siblings, 1 reply; 6+ messages in thread
From: Jason Yan @ 2023-04-18 14:05 UTC (permalink / raw
  To: Kanchan Joshi, David Howells; +Cc: hch, linux-block

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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: How best to get the size of a blockdev from a file?
  2023-04-18 10:00 ` How best to get the size of a blockdev from a file? David Howells
  2023-04-18 12:28   ` Kanchan Joshi
@ 2023-04-18 15:34   ` Christoph Hellwig
  1 sibling, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2023-04-18 15:34 UTC (permalink / raw
  To: David Howells; +Cc: hch, linux-block

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: How best to get the size of a blockdev from a file?
  2023-04-18 14:05     ` Jason Yan
@ 2023-04-20 19:04       ` Al Viro
  2023-04-21  1:38         ` Jason Yan
  0 siblings, 1 reply; 6+ messages in thread
From: Al Viro @ 2023-04-20 19:04 UTC (permalink / raw
  To: Jason Yan; +Cc: Kanchan Joshi, David Howells, hch, linux-block

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: How best to get the size of a blockdev from a file?
  2023-04-20 19:04       ` Al Viro
@ 2023-04-21  1:38         ` Jason Yan
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Yan @ 2023-04-21  1:38 UTC (permalink / raw
  To: Al Viro; +Cc: Kanchan Joshi, David Howells, hch, linux-block

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-04-21  1:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20230418100108epcas5p2d0f2a7a274e78731373986b3d4fced9b@epcas5p2.samsung.com>
2023-04-18 10:00 ` How best to get the size of a blockdev from a file? David Howells
2023-04-18 12:28   ` Kanchan Joshi
2023-04-18 14:05     ` Jason Yan
2023-04-20 19:04       ` Al Viro
2023-04-21  1:38         ` Jason Yan
2023-04-18 15:34   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).