| Message ID | 20250410184103.23385-3-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: discard alignment fixes | expand |
On 10.04.25 20:41, Stefan Hajnoczi wrote: > When guests send misaligned discard requests, the block layer breaks > them up into a misaligned head, an aligned main body, and a misaligned > tail. > > The file-posix block driver on Linux returns -EINVAL on misaligned > discard requests. This causes bdrv_co_pdiscard() to fail and guests > configured with werror=stop will pause. > > Add a special case for misaligned head/tail requests. Simply continue > when EINVAL is encountered so that the aligned main body of the request > can be completed and the guest is not paused. This is the best we can do > when guest discard limits do not match the host discard limits. > > Fixes: https://issues.redhat.com/browse/RHEL-86032 > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/io.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/block/io.c b/block/io.c > index 1ba8d1aeea..a0d0b31a3e 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -3180,7 +3180,11 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, > } > } > if (ret && ret != -ENOTSUP) { > - goto out; > + if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) { Could use `(offset | num) % align != 0`, but either way: Reviewed-by: Hanna Czenczek <hreitz@redhat.com> > + /* Silently skip rejected unaligned head/tail requests */ > + } else { > + goto out; /* bail out */ > + } > } > > offset += num;
On Fri, Apr 11, 2025 at 10:18:55AM +0200, Hanna Czenczek wrote: > > if (ret && ret != -ENOTSUP) { > > - goto out; > > + if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) { > > Could use `(offset | num) % align != 0`, but either way: Use of | and & to perform alignment checks only works if align is guaranteed to be a power of 2. But isn't there (odd) hardware out there with something like a 15M alignment, at which point you HAVE to do separate checks with % because bitwise ops no longer work?
On 11.04.25 19:28, Eric Blake wrote: > On Fri, Apr 11, 2025 at 10:18:55AM +0200, Hanna Czenczek wrote: >>> if (ret && ret != -ENOTSUP) { >>> - goto out; >>> + if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) { >> Could use `(offset | num) % align != 0`, but either way: > Use of | and & to perform alignment checks only works if align is > guaranteed to be a power of 2. But isn't there (odd) hardware out > there with something like a 15M alignment, at which point you HAVE to > do separate checks with % because bitwise ops no longer work? Ah, true, thanks! Hanna
diff --git a/block/io.c b/block/io.c index 1ba8d1aeea..a0d0b31a3e 100644 --- a/block/io.c +++ b/block/io.c @@ -3180,7 +3180,11 @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, } } if (ret && ret != -ENOTSUP) { - goto out; + if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) { + /* Silently skip rejected unaligned head/tail requests */ + } else { + goto out; /* bail out */ + } } offset += num;
When guests send misaligned discard requests, the block layer breaks them up into a misaligned head, an aligned main body, and a misaligned tail. The file-posix block driver on Linux returns -EINVAL on misaligned discard requests. This causes bdrv_co_pdiscard() to fail and guests configured with werror=stop will pause. Add a special case for misaligned head/tail requests. Simply continue when EINVAL is encountered so that the aligned main body of the request can be completed and the guest is not paused. This is the best we can do when guest discard limits do not match the host discard limits. Fixes: https://issues.redhat.com/browse/RHEL-86032 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/io.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)