Re: [PATCH v8 06/19] block, fs: Propagate write hints to the block device inode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/19/2024 7:26 PM, Kanchan Joshi wrote:
> On 1/19/2024 12:24 AM, Bart Van Assche wrote:
>> On 1/18/24 10:51, Kanchan Joshi wrote:
>>> Are you considering to change this so that hint is set only on one inode
>>> (and not on two)?
>>> IOW, should not this fragment be like below:
>>>
>>> --- a/fs/fcntl.c
>>> +++ b/fs/fcntl.c
>>> @@ -306,7 +306,6 @@ static long fcntl_get_rw_hint(struct file *file,
>>> unsigned int cmd,
>>>     static long fcntl_set_rw_hint(struct file *file, unsigned int cmd,
>>>                                  unsigned long arg)
>>>     {
>>> -       void (*apply_whint)(struct file *, enum rw_hint);
>>>            struct inode *inode = file_inode(file);
>>>            u64 __user *argp = (u64 __user *)arg;
>>>            u64 hint;
>>> @@ -316,11 +315,15 @@ static long fcntl_set_rw_hint(struct file *file,
>>> unsigned int cmd,
>>>            if (!rw_hint_valid(hint))
>>>                    return -EINVAL;
>>>
>>> +       /*
>>> +        * file->f_mapping->host may differ from inode. As an example
>>> +        * blkdev_open() modifies file->f_mapping
>>> +        */
>>> +       if (file->f_mapping->host != inode)
>>> +               inode = file->f_mapping->host;
>>> +
>>>            inode_lock(inode);
>>>            inode->i_write_hint = hint;
>>> -       apply_whint = inode->i_fop->apply_whint;
>>> -       if (apply_whint)
>>> -               apply_whint(file, hint);
>>>            inode_unlock(inode);
>>
>> I think the above proposal would introduce a bug: it would break the
>> F_GET_RW_HINT implementation.
> 
> Right. I expected to keep the exact change in GET, too, but that will
> not be free from the side-effect.
> The buffered-write path (block_write_full_page) picks the hint from one
> inode, and the direct-write path (__blkdev_direct_IO_simple) picks the
> hint from a different inode.
> So, updating both seems needed here.

I stand corrected. It's possible to do away with two updates.
The direct-io code (patch 8) should rather be changed to pick the hint 
from bdev inode (and not from file inode).
With that change, this patch only need to set the hint into only one 
inode (bdev one). What do you think?




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux