On Fri 15-12-17 09:17:42, Yan, Zheng wrote: > On Fri, Dec 15, 2017 at 12:53 AM, Jan Kara <jack@xxxxxxx> wrote: > >> > > >> > In this particular case I'm not sure why does ceph pass 'filp' into > >> > readpage() / readpages() handler when it already gets that pointer as part > >> > of arguments... > >> > >> It actually a flag which tells ceph_readpages() if its caller is > >> ceph_read_iter or readahead/fadvise/madvise. because when there are > >> multiple clients read/write a file a the same time, page cache should > >> be disabled. > > > > I'm not sure I understand the reasoning properly but from what you say > > above it rather seems the 'hint' should be stored in the inode (or possibly > > struct file)? > > > > The capability of using page cache is hold by the process who got it. > ceph_read_iter() first gets the capability, calls > generic_file_read_iter(), then release the capability. The capability > can not be easily stored in inode or file because it can be revoked by > server any time if caller does not hold it OK, understood. But using storage in task_struct (such as journal_info) is problematic as it has hard to fix recursion issues as the bug you're trying to fix shows (it is difficult to track down all the paths that can recurse into another filesystem which will clobber the stored info). So either you have to come up with some scheme to safely use current->journal_info (by somehow tracking owner as Andreas suggests) and convert all users to it or you have to come up with some scheme propagating the information through the inode / file->private_data and use it in Ceph. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
