On Thu, Sep 22, 2016 at 6:44 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> On Thu, Sep 22, 2016 at 5:49 PM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>> On Thu, Sep 22, 2016 at 10:49 AM, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>>> Guys, I just hit a lockdep warning on nesting an mnt_want_write() with
>>> this patch
>>> because overlay already holds the write lock when getting to ovl_copy_up_data()
>>> I don't know how I missed it before.
>>>
>>> Is it really a problem to nest this lock?
>>> Should I factor our another set of _locked helpers from the
>>> vfs_{copy,clone}_file_range helpers?
>>>
>>
>> vfs_{copy,clone}_file_range should call sb_start_write() instead of
>> mnt_want_write_file(), the checks against FMODE_WRITE + S_ISREG ensure
>> that the __mnt_want_write() part is already held on the file.
>
> Wait a minute, I think you got the solution but mixed it up a bit.
> IIUC, vfs_{clone,copy} should call __mnt_want_write_file()
> instead of mnt_want_write_file(), which will skip sb_start_write()
> because file is already open for write and no lockdep warning
> and no problem.
>
> Am I missing something?
>
Ah.. I got it. Anyway a short survey of fs/namei.c
shows that the custom is to have mnt_want_write() in either the syscall
or the do_XXX helper and not in the vfs_XXX helper,
so I will conform to this standard.
>>
>> That still leaves the lockdep warning. We could do
>> __vfs_{copy,clone}_file_range() variants without the
>> sb_start_write()/sb_end_write() and add the non-underscore variants as
>> static inline to fs.h that do call the sb_start/end_write.
>>
>> Thanks,
>> Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
