Re: [PATCH] fs: create kiocb_{start,end}_write() helpers

[Jens Axboe <axboe@kernel.dk>]

On 8/15/23 12:48 PM, Amir Goldstein wrote:
> On Tue, Aug 15, 2023 at 8:06?PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 8/15/23 11:02 AM, Jens Axboe wrote:
>>> On 8/15/23 10:57 AM, Amir Goldstein wrote:
>>>> +/**
>>>> + * kiocb_start_write - get write access to a superblock for async file io
>>>> + * @iocb: the io context we want to submit the write with
>>>> + *
>>>> + * This is a variant of file_start_write() for async io submission.
>>>> + * Should be matched with a call to kiocb_end_write().
>>>> + */
>>>> +static inline void kiocb_start_write(struct kiocb *iocb)
>>>> +{
>>>> +    struct inode *inode = file_inode(iocb->ki_filp);
>>>> +
>>>> +    iocb->ki_flags |= IOCB_WRITE;
>>>> +    if (WARN_ON_ONCE(iocb->ki_flags & IOCB_WRITE_STARTED))
>>>> +            return;
>>>> +    if (!S_ISREG(inode->i_mode))
>>>> +            return;
>>>> +    sb_start_write(inode->i_sb);
>>>> +    /*
>>>> +     * Fool lockdep by telling it the lock got released so that it
>>>> +     * doesn't complain about the held lock when we return to userspace.
>>>> +     */
>>>> +    __sb_writers_release(inode->i_sb, SB_FREEZE_WRITE);
>>>> +    iocb->ki_flags |= IOCB_WRITE_STARTED;
>>>> +}
>>>> +
>>>> +/**
>>>> + * kiocb_end_write - drop write access to a superblock after async file io
>>>> + * @iocb: the io context we sumbitted the write with
>>>> + *
>>>> + * Should be matched with a call to kiocb_start_write().
>>>> + */
>>>> +static inline void kiocb_end_write(struct kiocb *iocb)
>>>> +{
>>>> +    struct inode *inode = file_inode(iocb->ki_filp);
>>>> +
>>>> +    if (!(iocb->ki_flags & IOCB_WRITE_STARTED))
>>>> +            return;
>>>> +    if (!S_ISREG(inode->i_mode))
>>>> +            return;
>>
>> And how would IOCB_WRITE_STARTED ever be set, if S_ISREG() isn't true?
> 
> Good point.
> I will pass is_reg argument from callers of kiocb_start_write() and
> will only check IOCB_WRITE_STARTED in kiocb_end_write().

Please don't pass in an argument that just makes the function do
nothing. Just gate calling the function on it instead.

-- 
Jens Axboe