On Wed, Oct 13, 2021 at 05:28:36AM -0400, Mikulas Patocka wrote: > Hi > > Here I'm sending version 4 of the patch. It adds #include <linux/falloc.h> > to cifs and overlayfs to fix the bugs found out by the kernel test robot. > > Mikulas > > > > From: Mikulas Patocka <mpatocka@xxxxxxxxxx> > > The loop driver checks for the fallocate method and if it is present, it > assumes that the filesystem can do FALLOC_FL_ZERO_RANGE and > FALLOC_FL_PUNCH_HOLE requests. However, some filesystems (such as fat, or > tmpfs) have the fallocate method, but lack the capability to do > FALLOC_FL_ZERO_RANGE and/or FALLOC_FL_PUNCH_HOLE. This seems like a loopback driver level problem, not something filesystems need to solve. fallocate() is defined to return -EOPNOTSUPP if a flag is passed that it does not support and that's the mechanism used to inform callers that a fallocate function is not supported by the underlying filesystem/storage. Indeed, filesystems can support hole punching at the ->fallocate(), but then return EOPNOTSUPP because certain dynamic conditions are not met e.g. CIFS needs sparse file support on the server to support hole punching, but we don't know this until we actually try to sparsify the file. IOWs, this patch doesn't address all the cases where EOPNOTSUPP might actually get returned from filesystems and/or storage. > This results in syslog warnings "blk_update_request: operation not > supported error, dev loop0, sector 0 op 0x9:(WRITE_ZEROES) flags 0x800800 > phys_seg 0 prio class 0". The error can be reproduced with this command: > "truncate -s 1GiB /tmp/file; losetup /dev/loop0 /tmp/file; blkdiscard -z > /dev/loop0" Which I'm assuming comes from this: if (unlikely(error && !blk_rq_is_passthrough(req) && !(req->rq_flags & RQF_QUIET))) print_req_error(req, error, __func__); Which means we could supress the error message quite easily in lo_fallocate() by doing: out: if (ret == -EOPNOTSUPP) rq->rq_flags |= RQF_QUIET; return ret; And then we can also run blk_queue_flag_clear(QUEUE_FLAG_DISCARD) (and whatever else is needed to kill discards) to turn off future discard attempts on that loopback device. This way the problem is just quietly and correctly handled by the loop device and everything is good... Thoughts? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel