Jan Kara <jack@xxxxxxx> writes: > On Tue 28-08-18 13:56:30, Mike Snitzer wrote: >> On Tue, Aug 28 2018 at 3:50am -0400, >> Jan Kara <jack@xxxxxxx> wrote: >> >> > On Mon 27-08-18 16:43:28, Kani, Toshi wrote: >> > > On Mon, 2018-08-27 at 18:07 +0200, Jan Kara wrote: >> > > > Hi, >> > > > >> > > > I've been analyzing why fstest generic/081 fails when the backing device is >> > > > capable of DAX. The problem boils down to the failure of: >> > > > >> > > > lvm vgcreate -f vg0 /dev/pmem0 >> > > > lvm lvcreate -L 128M -n lv0 vg0 >> > > > lvm lvcreate -s -L 4M -n snap0 vg0/lv0 >> > > > >> > > > The last command fails like: >> > > > >> > > > device-mapper: reload ioctl on (253:0) failed: Invalid argument >> > > > Failed to lock logical volume vg0/lv0. >> > > > Aborting. Manual intervention required. >> > > > >> > > > And the core of the problem is that volume vg0/lv0 is originally of >> > > > DM_TYPE_DAX_BIO_BASED type but when the snapshot gets created, we try to >> > > > switch it to DM_TYPE_BIO_BASED because now the device stops supporting DAX. >> > > > The problem seems to be introduced by Ross' commit dbc626597 "dm: prevent >> > > > DAX mounts if not supported". >> > > > >> > > > The question is whether / how this should be fixed. The current inability >> > > > to create snapshots of DAX-capable devices looks weird and the cryptic >> > > > failure makes it even worse (it took me quite a while to understand what is >> > > > failing and why). OTOH I see the rationale behind Ross' change as well. >> > > >> > > Here are the dm-snap changes that went along with the original DAX >> > > support. >> > > >> > > commit b5ab4a9ba55 >> > > commit f6e629bd237 >> > > >> > > Basically, snapshots can be added/removed to DAX-capable devices, but >> > > snapshots need to be mounted without dax option. >> > >> > Yes, and after these two commits things were working. But then commit >> > dbc626597 broke things again so currently snapshotting DAX-capable devices >> > does not work. Just try with 4.18... >> >> Commit f6e629bd237 was a nasty hack, and commit dbc626597 exposed it as >> such. But commit dbc626597 has caused us to regress.. so we need to fix >> it. >> >> We could remove DM_TYPE_DAX_BIO_BASED completely. But in the past I was >> reluctant to do so because it really is unclear how/if we can even >> support a device switching from DAX to non-DAX while IO is in-flight. DM >> supports suspending without flushing (via dmsetup suspend --noflush) and >> that could really be problematic if we leave DAX IO inflight and then >> switch the DM table such that the DM device no longer supports DAX. > > Well, changing device from DAX-capable to DAX-incapable is problematic for > filesystem on top of it as well. Filesystems simply don't expect this > feature of a device can change so they would fail in unexpected ways. Also > PFNs from the pmem (DAX-capable) device that are already mapped to user page > tables won't magically become unmapped so those processes will still have > DAX access to those areas of the device. > > But, if both original bdev and COW device are DAX-capable, we *should* be > able to support snapshotting (and refusing mixing of DAX-capable and > DAX-incapable devices in a snapshot is IMHO not very surprising to users). > When creating a snapshot of a device, we need to freeze the filesystem > using it. That will writeprotect all page tables so we are sure we'll get > page faults (and thus ->direct_access requests from DM POV) for each write > attempt to any mapping. Then ->direct_access method of snapshot-origin can > make sure to copy original contents to the COW-device before returning PFN > from ->direct_access. Similarly ->direct_access of COW-device can provide > remapped PFN so everything should work seamlessly from user POV. In your example above, if two processes have a file mapped with MAP_SHARED, and P1 does a store, the new contents will not be reflected in P2, right?. This is different from what is expected, and different from what happens when the page cache is involved. I think you'd need to unmap all mappings on a CoW, whether triggered by a store to an existing mapping or a write(2). -Jeff
