| Message ID | 20230929102726.2985188-10-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | block atomic writes | expand |
Hi John, kernel test robot noticed the following build errors: [auto build test ERROR on xfs-linux/for-next] [also build test ERROR on axboe-block/for-next mkp-scsi/for-next jejb-scsi/for-next linus/master v6.6-rc3 next-20230929] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/John-Garry/block-Add-atomic-write-operations-to-request_queue-limits/20230929-184542 base: https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next patch link: https://lore.kernel.org/r/20230929102726.2985188-10-john.g.garry%40oracle.com patch subject: [PATCH 09/21] block: Add checks to merging of atomic writes config: mips-mtx1_defconfig (https://download.01.org/0day-ci/archive/20230930/202309302100.L6ynQWub-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230930/202309302100.L6ynQWub-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309302100.L6ynQWub-lkp@intel.com/ All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: __moddi3 >>> referenced by blk-merge.c >>> block/blk-merge.o:(ll_back_merge_fn) in archive vmlinux.a >>> referenced by blk-merge.c >>> block/blk-merge.o:(ll_back_merge_fn) in archive vmlinux.a >>> referenced by blk-merge.c >>> block/blk-merge.o:(bio_attempt_front_merge) in archive vmlinux.a >>> referenced 3 more times
On Sat, Sep 30, 2023 at 09:40:30PM +0800, kernel test robot wrote: > Hi John, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on xfs-linux/for-next] > [also build test ERROR on axboe-block/for-next mkp-scsi/for-next jejb-scsi/for-next linus/master v6.6-rc3 next-20230929] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/John-Garry/block-Add-atomic-write-operations-to-request_queue-limits/20230929-184542 > base: https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next > patch link: https://lore.kernel.org/r/20230929102726.2985188-10-john.g.garry%40oracle.com > patch subject: [PATCH 09/21] block: Add checks to merging of atomic writes > config: mips-mtx1_defconfig (https://download.01.org/0day-ci/archive/20230930/202309302100.L6ynQWub-lkp@intel.com/config) > compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230930/202309302100.L6ynQWub-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202309302100.L6ynQWub-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > >> ld.lld: error: undefined symbol: __moddi3 > >>> referenced by blk-merge.c > >>> block/blk-merge.o:(ll_back_merge_fn) in archive vmlinux.a > >>> referenced by blk-merge.c > >>> block/blk-merge.o:(ll_back_merge_fn) in archive vmlinux.a > >>> referenced by blk-merge.c > >>> block/blk-merge.o:(bio_attempt_front_merge) in archive vmlinux.a > >>> referenced 3 more times This does not appear to be clang specific, I can reproduce it with GCC 12.3.0 and the same configuration target. Cheers, Nathan
On 02/10/2023 23:50, Nathan Chancellor wrote: >>>> ld.lld: error: undefined symbol: __moddi3 >> >>> referenced by blk-merge.c >> >>> block/blk-merge.o:(ll_back_merge_fn) in archive vmlinux.a >> >>> referenced by blk-merge.c >> >>> block/blk-merge.o:(ll_back_merge_fn) in archive vmlinux.a >> >>> referenced by blk-merge.c >> >>> block/blk-merge.o:(bio_attempt_front_merge) in archive vmlinux.a >> >>> referenced 3 more times > This does not appear to be clang specific, I can reproduce it with GCC > 12.3.0 and the same configuration target. Yeah, I just need to stop using the modulo operator for 64b values, which I had already been advised to :| Thanks, John
diff --git a/block/blk-merge.c b/block/blk-merge.c index bc21f8ff4842..5dc850924e29 100644 --- a/block/blk-merge.c +++ b/block/blk-merge.c @@ -18,6 +18,23 @@ #include "blk-rq-qos.h" #include "blk-throttle.h" +static bool bio_straddles_atomic_write_boundary(loff_t bi_sector, + unsigned int bi_size, + unsigned int boundary) +{ + loff_t start = bi_sector << SECTOR_SHIFT; + loff_t end = start + bi_size; + loff_t start_mod = start % boundary; + loff_t end_mod = end % boundary; + + if (end - start > boundary) + return true; + if ((start_mod > end_mod) && (start_mod && end_mod)) + return true; + + return false; +} + static inline void bio_get_first_bvec(struct bio *bio, struct bio_vec *bv) { *bv = mp_bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter); @@ -664,6 +681,18 @@ int ll_back_merge_fn(struct request *req, struct bio *bio, unsigned int nr_segs) return 0; } + if (req->cmd_flags & REQ_ATOMIC) { + unsigned int atomic_write_boundary_bytes = + queue_atomic_write_boundary_bytes(req->q); + + if (atomic_write_boundary_bytes && + bio_straddles_atomic_write_boundary(req->__sector, + bio->bi_iter.bi_size + blk_rq_bytes(req), + atomic_write_boundary_bytes)) { + return 0; + } + } + return ll_new_hw_segment(req, bio, nr_segs); } @@ -683,6 +712,19 @@ static int ll_front_merge_fn(struct request *req, struct bio *bio, return 0; } + if (req->cmd_flags & REQ_ATOMIC) { + unsigned int atomic_write_boundary_bytes = + queue_atomic_write_boundary_bytes(req->q); + + if (atomic_write_boundary_bytes && + bio_straddles_atomic_write_boundary( + bio->bi_iter.bi_sector, + bio->bi_iter.bi_size + blk_rq_bytes(req), + atomic_write_boundary_bytes)) { + return 0; + } + } + return ll_new_hw_segment(req, bio, nr_segs); } @@ -719,6 +761,18 @@ static int ll_merge_requests_fn(struct request_queue *q, struct request *req, blk_rq_get_max_sectors(req, blk_rq_pos(req))) return 0; + if (req->cmd_flags & REQ_ATOMIC) { + unsigned int atomic_write_boundary_bytes = + queue_atomic_write_boundary_bytes(req->q); + + if (atomic_write_boundary_bytes && + bio_straddles_atomic_write_boundary(req->__sector, + blk_rq_bytes(req) + blk_rq_bytes(next), + atomic_write_boundary_bytes)) { + return 0; + } + } + total_phys_segments = req->nr_phys_segments + next->nr_phys_segments; if (total_phys_segments > blk_rq_get_max_segments(req)) return 0; @@ -814,6 +868,18 @@ static enum elv_merge blk_try_req_merge(struct request *req, return ELEVATOR_NO_MERGE; } +static bool blk_atomic_write_mergeable_rq_bio(struct request *rq, + struct bio *bio) +{ + return (rq->cmd_flags & REQ_ATOMIC) == (bio->bi_opf & REQ_ATOMIC); +} + +static bool blk_atomic_write_mergeable_rqs(struct request *rq, + struct request *next) +{ + return (rq->cmd_flags & REQ_ATOMIC) == (next->cmd_flags & REQ_ATOMIC); +} + /* * For non-mq, this has to be called with the request spinlock acquired. * For mq with scheduling, the appropriate queue wide lock should be held. @@ -833,6 +899,9 @@ static struct request *attempt_merge(struct request_queue *q, if (req->ioprio != next->ioprio) return NULL; + if (!blk_atomic_write_mergeable_rqs(req, next)) + return NULL; + /* * If we are allowed to merge, then append bio list * from next to rq and release next. merge_requests_fn @@ -960,6 +1029,9 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio) if (rq->ioprio != bio_prio(bio)) return false; + if (blk_atomic_write_mergeable_rq_bio(rq, bio) == false) + return false; + return true; }
For atomic writes we allow merging, but we must adhere to some additional rules: - Only allow merging of atomic writes with other atomic writes - Ensure that the merged IO would not cross an atomic write boundary, if any We already ensure that we don't exceed the atomic writes size limit in get_max_io_size(). Signed-off-by: John Garry <john.g.garry@oracle.com> --- block/blk-merge.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)