"Darrick J. Wong" <djwong@xxxxxxxxxx> writes: > On Tue, Jun 11, 2024 at 12:13:22PM +0530, Ritesh Harjani wrote: >> "Darrick J. Wong" <djwong@xxxxxxxxxx> writes: >> <snip> >> >> > + * ``addr`` describes the device address, in bytes. >> >> > + >> >> > + * ``bdev`` describes the block device for this mapping. >> >> > + This only needs to be set for mapped or unwritten operations. >> >> > + >> >> > + * ``dax_dev`` describes the DAX device for this mapping. >> >> > + This only needs to be set for mapped or unwritten operations, and >> >> > + only for a fsdax operation. >> >> >> >> Looks like we can make this union (bdev and dax_dev). Since depending >> >> upon IOMAP_DAX or not we only set either dax_dev or bdev. >> > >> > Separate patch. ;) >> > >> >> Yes, in a way I was trying to get an opinion from you and others on >> whether it make sense to make bdev and dax_dev as union :) >> >> Looks like this series [1] could be the reason for that. >> >> [1]: https://lore.kernel.org/all/20211129102203.2243509-1-hch@xxxxxx/#t >> >> I also don't see any reference to dax code from fs/iomap/buffered-io.c >> So maybe we don't need this dax.h header in this file. >> >> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c >> index c5802a459334..e1a6cca3cec2 100644 >> --- a/fs/iomap/buffered-io.c >> +++ b/fs/iomap/buffered-io.c >> @@ -10,7 +10,6 @@ >> #include <linux/pagemap.h> >> #include <linux/uio.h> >> #include <linux/buffer_head.h> >> -#include <linux/dax.h> >> #include <linux/writeback.h> >> #include <linux/list_sort.h> >> #include <linux/swap.h> > > Yes, given that both you and hch have mentioned it, could one of you > send a cleanup series for that? > Sure, Thanks Darrick and Christoph. I can queue this with my other work where I am improving iomap for indirect-block mapping, so that it will be easier to get testing done on all of this together. -ritesh