| Message ID | cover.1682069716.git.ritesh.list@gmail.com |
|---|---|
| Headers | show |
| Series | ext2: DIO to use iomap | expand |
Hello Ritesh, On Fri 21-04-23 15:16:10, Ritesh Harjani (IBM) wrote: > Hello All, > > Please find the series which rewrites ext2 direct-io path to use modern > iomap interface. The patches now all look good to me. I'd like to discuss a bit how to merge them. The series has an ext4 cleanup (patch 3) and three iomap patches (patches 6, 8 and 9). Darrick, do you want to take the iomap patches through your tree? The only dependency is that patch 7 for ext2 is dependent on definitions from patch 6 so I'd have to pull your branch into my tree. Or I can take all the iomap patches through my tree but for that it would be nice to have Darrick's acks. I can take the ext4 patch through my tree unless Ted objects. I guess I won't rush this for the coming merge window (unless Linus decides to do rc8) but once we settle on the merge strategy I'll push out some branch on which we can base further ext2 iomap conversion work. Honza > PATCHv5 -> PATCHv6: > =================== > 1. Patch-2 Added generic_buffers_fsync_noflush() & generic_buffers_fsync() functions. > 2. Patch-3 & Patch-4 to use above functions in ext4 & ext2. > 3. Added Reviewed-by from Christoph on Patch-9 (iomap: Add DIO tracepoints) > > RFCv4 -> PATCHv5: > ================= > 1. Added trace_iomap_dio_rw_begin tracepoint in __iomap_dio_rw() > 2. Added Reviewed-by tags from Christoph > > RFCv3 -> RFCV4: > =============== > 1. Renamed __generic_file_fsync_nolock() from libfs to generic_buffer_fsync() in > fs/buffer.c > (Review comment from Christoph) > 2. Fixed s/EVENTD/EVENTFD/ in TRACE_IOCB_STRINGS > 3. Fixed few data types for parameters in ext2 trace patch (size_t && ssize_t) > 4. Killed this patch "Minor refactor of iomap_dio_rw" > 5. Changed iomap tracepoint patch and fixed the data types (size_t && ssize_t) > (addressed review comments from Christoph) > > RFCv2 -> RFCv3: > =============== > 1. Addressed minor review comments related to extern, parameter naming in > function declaration, removing not required braces and shorting overly long > lines. > 2. Added Reviewed-by from various reviewers. > 3. Fixed a warning & couple of compilation errors in Patch-7 (ext2 trace points) > related to CFLAGS_trace & second related to unable to find function > definition for iov_iter_count(). (requires uio.h file) > CFLAGS_trace is required in Makefile so that it can find trace.h file from > tracepoint infrastructure. > 4. Changed naming of IOCB_STRINGS TO TRACE_IOCB_STRINGS. > 5. Shortened naming of tracepoint events for ext2 dio. > 6. Added iomap DIO tracepoint events. > 7. Disha tested this series internally against Power with "auto" group for 4k > and 64k blocksize configuration. Added her "Tested-by" tag in all DIO > related patches. No new failures were reported. > > Thanks everyone for the review and test. The series is looking good to me now. > It has been tested on x86 and Power with different configurations. > Please let me know if anything else is required on this. > > v2: https://lore.kernel.org/all/ZDTybcM4kjYLSrGI@infradead.org/ > > Ritesh Harjani (IBM) (9): > ext2/dax: Fix ext2_setsize when len is page aligned > fs/buffer.c: Add generic_buffers_fsync*() implementation > ext4: Use generic_buffers_fsync_noflush() implementation > ext2: Use generic_buffers_fsync() implementation > ext2: Move direct-io to use iomap > fs.h: Add TRACE_IOCB_STRINGS for use in trace points > ext2: Add direct-io trace points > iomap: Remove IOMAP_DIO_NOSYNC unused dio flag > iomap: Add DIO tracepoints > > fs/buffer.c | 70 ++++++++++++++++++++ > fs/ext2/Makefile | 5 +- > fs/ext2/ext2.h | 1 + > fs/ext2/file.c | 126 +++++++++++++++++++++++++++++++++++- > fs/ext2/inode.c | 58 ++++++++++------- > fs/ext2/trace.c | 6 ++ > fs/ext2/trace.h | 94 +++++++++++++++++++++++++++ > fs/ext4/fsync.c | 33 +++++----- > fs/iomap/direct-io.c | 9 ++- > fs/iomap/trace.c | 1 + > fs/iomap/trace.h | 78 ++++++++++++++++++++++ > include/linux/buffer_head.h | 4 ++ > include/linux/fs.h | 14 ++++ > include/linux/iomap.h | 6 -- > 14 files changed, 456 insertions(+), 49 deletions(-) > create mode 100644 fs/ext2/trace.c > create mode 100644 fs/ext2/trace.h > > -- > 2.39.2 >
Jan Kara <jack@suse.cz> writes: > Hello Ritesh, > > On Fri 21-04-23 15:16:10, Ritesh Harjani (IBM) wrote: >> Hello All, >> >> Please find the series which rewrites ext2 direct-io path to use modern >> iomap interface. > > The patches now all look good to me. I'd like to discuss a bit how to merge Thanks Jan, > them. The series has an ext4 cleanup (patch 3) and three iomap patches Also Patch-3 is on top of ext4 journalled data patch series of yours, otheriwse we might see a minor merge conflict. https://lore.kernel.org/all/20230329154950.19720-6-jack@suse.cz/ > (patches 6, 8 and 9). Darrick, do you want to take the iomap patches through > your tree? > > The only dependency is that patch 7 for ext2 is dependent on definitions > from patch 6 That's right. Patch 6 defines TRACE_IOCB_STRINGS definition which both ext2 and iomap tracepoints depend upon. > so I'd have to pull your branch into my tree. Or I can take > all the iomap patches through my tree but for that it would be nice to have > Darrick's acks. > > I can take the ext4 patch through my tree unless Ted objects. Sure, we might have to merge with Ted's ext4 tree as well to avoid the merge conflict I mentioned above. > > I guess I won't rush this for the coming merge window (unless Linus decides > to do rc8) but once we settle on the merge strategy I'll push out some Ok. > branch on which we can base further ext2 iomap conversion work. > Sure, will this branch also gets reflected in linux-next for wider testing? -ritesh
On Fri, Apr 21, 2023 at 05:35:47PM +0530, Ritesh Harjani wrote: > Jan Kara <jack@suse.cz> writes: > > > Hello Ritesh, > > > > On Fri 21-04-23 15:16:10, Ritesh Harjani (IBM) wrote: > >> Hello All, > >> > >> Please find the series which rewrites ext2 direct-io path to use modern > >> iomap interface. > > > > The patches now all look good to me. I'd like to discuss a bit how to merge > > Thanks Jan, > > > > them. The series has an ext4 cleanup (patch 3) and three iomap patches > > Also Patch-3 is on top of ext4 journalled data patch series of yours, > otheriwse we might see a minor merge conflict. > > https://lore.kernel.org/all/20230329154950.19720-6-jack@suse.cz/ > > > (patches 6, 8 and 9). Darrick, do you want to take the iomap patches through > > your tree? Hmm. I could do that for 6.4 since the first one should be trivially verifiable and so far Linus hasn't objected to patches that add tracepoints being thrown into for-next right at the start of the merge window. --D > > The only dependency is that patch 7 for ext2 is dependent on definitions > > from patch 6 > > That's right. Patch 6 defines TRACE_IOCB_STRINGS definition which both > ext2 and iomap tracepoints depend upon. > > > so I'd have to pull your branch into my tree. Or I can take > > all the iomap patches through my tree but for that it would be nice to have > > Darrick's acks. > > > > I can take the ext4 patch through my tree unless Ted objects. > > Sure, we might have to merge with Ted's ext4 tree as well to avoid the > merge conflict I mentioned above. > > > > > I guess I won't rush this for the coming merge window (unless Linus decides > > to do rc8) but once we settle on the merge strategy I'll push out some > > Ok. > > > branch on which we can base further ext2 iomap conversion work. > > > > Sure, will this branch also gets reflected in linux-next for wider testing? > > -ritesh
On Fri 21-04-23 08:40:58, Darrick J. Wong wrote: > On Fri, Apr 21, 2023 at 05:35:47PM +0530, Ritesh Harjani wrote: > > Jan Kara <jack@suse.cz> writes: > > > > > Hello Ritesh, > > > > > > On Fri 21-04-23 15:16:10, Ritesh Harjani (IBM) wrote: > > >> Hello All, > > >> > > >> Please find the series which rewrites ext2 direct-io path to use modern > > >> iomap interface. > > > > > > The patches now all look good to me. I'd like to discuss a bit how to merge > > > > Thanks Jan, > > > > > > > them. The series has an ext4 cleanup (patch 3) and three iomap patches > > > > Also Patch-3 is on top of ext4 journalled data patch series of yours, > > otheriwse we might see a minor merge conflict. > > > > https://lore.kernel.org/all/20230329154950.19720-6-jack@suse.cz/ > > > > > (patches 6, 8 and 9). Darrick, do you want to take the iomap patches through > > > your tree? > > Hmm. I could do that for 6.4 since the first one should be trivially > verifiable and so far Linus hasn't objected to patches that add > tracepoints being thrown into for-next right at the start of the merge > window. Great! Then I'll wait a bit until rc1 when your iomap tree + Ted's ext4 tree should have landed in Linus' tree and push the ext2+ext4 patches into my tree to a branch based on rc1 which should deal with all the merge troubles. Honza
On Fri 21-04-23 17:35:47, Ritesh Harjani wrote: > Jan Kara <jack@suse.cz> writes: > > them. The series has an ext4 cleanup (patch 3) and three iomap patches > > Also Patch-3 is on top of ext4 journalled data patch series of yours, > otheriwse we might see a minor merge conflict. > > https://lore.kernel.org/all/20230329154950.19720-6-jack@suse.cz/ Yes, I have noticed :) > > I guess I won't rush this for the coming merge window (unless Linus decides > > to do rc8) but once we settle on the merge strategy I'll push out some > > Ok. > > > branch on which we can base further ext2 iomap conversion work. > > > > Sure, will this branch also gets reflected in linux-next for wider testing? Yes, the branch I'll push the ext2+ext4 patches to will be included in linux-next (I'll merge it to for-next branch). Honza