[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Nbd] [Qemu-devel] [PATCH] nbd: fix trim/discard comm

From: Michał Belczyk
Subject: Re: [Qemu-trivial] [Nbd] [Qemu-devel] [PATCH] nbd: fix trim/discard commands with a length bigger than NBD_MAX_BUFFER_SIZE
Date: Tue, 10 May 2016 21:13:24 +0200
User-agent: Mutt/1.6.1 (2016-04-27)

On Tue, May 10, 2016 at 04:41:27PM +0100, Alex Bligh wrote:

On 10 May 2016, at 16:29, Eric Blake <address@hidden> wrote:

So the kernel is currently one of the clients that does NOT honor block
sizes, and as such, servers should be prepared for ANY size up to
UINT_MAX (other than DoS handling).

Interesting followup question:

If the kernel does not fragment TRIM requests at all (in the
same way it fragments read and write requests), I suspect
something bad may happen with TRIM requests over 2^31
in size (particularly over 2^32 in size), as the length
field in nbd only has 32 bits.

Whether it supports block size constraints or not, it is
going to need to do *some* breaking up of requests.

Doesn't the kernel break TRIM requests at max_discard_sectors?

I remember testing the following change in the nbd kmod long time ago:

#if 0
                disk->queue->limits.max_discard_sectors = UINT_MAX;
                disk->queue->limits.max_discard_sectors = 65536;

The problem with large TRIM requests is exactly the same as with other (READ/WRITE) large requests -- they _may_ take loads of time and if the client wants to support a fast switch over to another replica it must put some constraints on the timeout value... which may be very easily broken if you allow things like a 1GB trim request on the server using DIO on the other side (and say heavily fragmented sparse file over XFS, never mind).

I don't think it's the problem of QEMU NBD server to fix that, the constraint on the server side is perfectly fine, the problem is to note that a change on the client side is required to limit the maximum size of the TRIM requests. The reason for the patch I pasted above was that at the time I looked into it there was no other way to change the TRIM request size via /sys/block/$dev/queue/max_discard_sectors, but maybe the more recent kernels allow that? Not to mention that the NBD client option to set that at NBD queue setup time would be nice...

Just my 2p.

Michał Belczyk Snr

reply via email to

[Prev in Thread] Current Thread [Next in Thread]