lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 21 Aug 2017 13:57:51 -0600 From: Ross Zwisler <ross.zwisler@...ux.intel.com> To: Dan Williams <dan.j.williams@...el.com> Cc: Boaz Harrosh <boazh@...app.com>, linux-xfs@...r.kernel.org, Jan Kara <jack@...e.cz>, linux-nvdimm <linux-nvdimm@...ts.01.org>, Dave Chinner <david@...morbit.com>, Christoph Hellwig <hch@...radead.org>, Andy Lutomirski <luto@...nel.org>, Linux FS Devel <linux-fsdevel@...r.kernel.org>, "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>, Amit Golander <amitg@...app.com> Subject: Re: [RFC PATCH 0/7] dax, ext4: Synchronous page faults On Mon, Aug 14, 2017 at 09:03:59AM -0700, Dan Williams wrote: > On Mon, Aug 14, 2017 at 7:04 AM, Boaz Harrosh <boazh@...app.com> wrote: > > > > Thank you Jan, I'm patiently waiting for this MAP_SYNC flag since I asked for > > it in 2014. I'm so glad its time is finally do. > > > > Thank you for working on this. Please CC me on future patches. > > (note the new Netapp email) > > > > On 13/08/17 12:25, Christoph Hellwig wrote: > >> On Sat, Aug 12, 2017 at 07:44:14PM -0700, Dan Williams wrote: > >>> How about MAP_SYNC == (MAP_SHARED|MAP_PRIVATE)? On older kernels that > >>> should get -EINVAL, and on new kernels it means SYNC+SHARED. > >> > >> Cute trick, but I'd hate to waster it just for our little flag. > >> > >> How about: > >> > >> #define __MAP_VALIDATE MAP_SHARED|MAP_PRIVATE > >> #define MAP_SYNC 0x??? | __MAP_VALIDATE > >> > >> so that we can reuse that trick for any new flag? > >> > > > > YES! And please create a mask for all new flags and in validation > > code if ((m_flags & __MAP_VALIDATE) == __MAP_VALIDATE) then you > > want that (m_flags & __MAP_NEWFLAGS) does not come empty, this > > way you actually preserve the old check that SHARED and PRIVATE > > do not co exist. > > > > Few Comments on this new MAP_ flag > > > > 0] The name at least needs to be MAP_MSYNC because only meta-data is > > synced not the data pointed to. That is the responsibility of the app > > > > 1] This flag you have named MAP_SYNC but it is very much related to > > dax and the ability for user-mode to "flush" the data pointed by this > > now "synced" meta data. > > For example in ext4, this flag set on an inode that is *not* IS_DAX > > should fail the mmap. Because there is no point of synced meta if the > > data is actually in page-cache and we know for sure it was not yet synced, > > And there is no way for user-mode to directly "sync" the data as well. > > > > 2] The code should be constructed that the default check for the MAP_SYNC > > should fail, and only Hopped in FSs are allowed. > > (So not to modify all Implementations of file_operations->mmap() ) > > > > 3] /dev/pmem could start serving DAX pages in mmap, if asked for MAP_MSYNC > > (which is also an API that says "I know I need to cl_flush". See 1. ) > > > > 4] Once we have this flag. And properly implemented at least in one FS > > and optionally in /dev/pmemX we no longer have any justification for > > /dev/daxX and it can die a slow and happy death. > > I'm all for replacing /dev/dax with filesystem equivalent > functionality, but I don't think MAP_SYNC gets us fully there. That's > what the MAP_DIRECT proposal [1] is meant to address. > > [1]: https://lkml.org/lkml/2017/8/13/160 I think we may also need to get DAX working on raw block devices again before /dev/dax can go away. AFAIK some customers still prefer to use DAX on a per-device basis and not have a filesystem.
Powered by blists - more mailing lists