From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 336F6149DF5 for ; Wed, 3 Apr 2024 15:04:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712156655; cv=none; b=Se8dNdFBvnS85FjSg7lcU31wiGl+NqqG7+lWjPob/hPKkCvxcbukmRH6nnkfzxKb9W6pg0rgq/g9+8GyYpPOdFGs21DcSuICZlTN8ToDt/wN8F6n+2lJNm56eiRrND9jXI5DyPT19Tl+MODdLIWeLOTgYGmv5OcCqA2vWH3e5u8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712156655; c=relaxed/simple; bh=k8uce1rf56gQTid1JBeAXtJeS2WnwUoqGN9N80BabCI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JpZh3eHaLCDB9cH9mwsIcESGKyy17xTu6DnTNVSU0zuU17km6ITmIrIfb3yVnVvgWE9djqdiTzpPiAR3AE6Xpfntmsawc7kRVSfkyWrEpmIqSqyKxKFdGsWYirgHIEqXtA0/1HlgQI/WarqKMJAwd/QcgtRnheUQOI3oA4cJai0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=Y2/jELtl; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Y2/jELtl" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1712156653; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=t/jx0gVWX+z6DL+RdUWe4Xtrtk69fV0JhZ/qa15t2TM=; b=Y2/jELtl5cParLi1fncpSSrUUwlrKNa0U29lsMnNgnGaBsn9JdKUKvVXudmEKqS7conA45 wzntW4r1sPXi1VVPc3KfPOhRVLqa1dsIlaAm5fSUt/vYaJObXSJ3QHSBCjo8I43ovlgIy6 yVREjRN7nZNq8WNrqOcBQ7wzeN0j7fw= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-368-2cBnbDAkODaeNogeLgoMow-1; Wed, 03 Apr 2024 11:04:09 -0400 X-MC-Unique: 2cBnbDAkODaeNogeLgoMow-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8A4273816B4D; Wed, 3 Apr 2024 15:04:08 +0000 (UTC) Received: from localhost (unknown [10.39.194.118]) by smtp.corp.redhat.com (Postfix) with ESMTP id CA7E64073484; Wed, 3 Apr 2024 15:04:07 +0000 (UTC) Date: Wed, 3 Apr 2024 11:03:46 -0400 From: Stefan Hajnoczi To: Eric Blake Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, Alasdair Kergon , Mikulas Patocka , dm-devel@lists.linux.dev, David Teigland , Mike Snitzer , Jens Axboe , Christoph Hellwig , Joe Thornber Subject: Re: [RFC 8/9] dm thin: add llseek(SEEK_HOLE/SEEK_DATA) support Message-ID: <20240403150346.GH2524049@fedora> References: <20240328203910.2370087-1-stefanha@redhat.com> <20240328203910.2370087-9-stefanha@redhat.com> Precedence: bulk X-Mailing-List: linux-block@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="RRfDDrHk+pzHnTwS" Content-Disposition: inline In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.2 --RRfDDrHk+pzHnTwS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 28, 2024 at 08:31:21PM -0500, Eric Blake wrote: > On Thu, Mar 28, 2024 at 04:39:09PM -0400, Stefan Hajnoczi wrote: > > Signed-off-by: Stefan Hajnoczi > > --- > > Open issues: > > - Locking? > > - thin_seek_hole_data() does not run as a bio or request. This patch > > assumes dm_thin_find_mapped_range() synchronously performs I/O if > > metadata needs to be loaded from disk. Is that a valid assumption? > > --- > > drivers/md/dm-thin.c | 77 ++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 77 insertions(+) > >=20 > > diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c > > index 4793ad2aa1f7e..3c5dc4f0fe8a3 100644 > > --- a/drivers/md/dm-thin.c > > +++ b/drivers/md/dm-thin.c > > @@ -4501,6 +4501,82 @@ static void thin_io_hints(struct dm_target *ti, = struct queue_limits *limits) > > } > > } > > =20 > > +static dm_block_t loff_to_block(struct pool *pool, loff_t offset) > > +{ > > + sector_t offset_sectors =3D offset >> SECTOR_SHIFT; > > + dm_block_t ret; > > + > > + if (block_size_is_power_of_two(pool)) > > + ret =3D offset_sectors >> pool->sectors_per_block_shift; > > + else { > > + ret =3D offset_sectors; > > + (void) sector_div(ret, pool->sectors_per_block); > > + } > > + > > + return ret; > > +} > > + > > +static loff_t block_to_loff(struct pool *pool, dm_block_t block) > > +{ > > + return block_to_sectors(pool, block) << SECTOR_SHIFT; > > +} > > + > > +static loff_t thin_seek_hole_data(struct dm_target *ti, loff_t offset, > > + int whence) > > +{ > > + struct thin_c *tc =3D ti->private; > > + struct dm_thin_device *td =3D tc->td; > > + struct pool *pool =3D tc->pool; > > + dm_block_t begin; > > + dm_block_t end; > > + dm_block_t mapped_begin; > > + dm_block_t mapped_end; > > + dm_block_t pool_begin; > > + bool maybe_shared; > > + int ret; > > + > > + /* TODO locking? */ > > + > > + if (block_size_is_power_of_two(pool)) > > + end =3D ti->len >> pool->sectors_per_block_shift; > > + else { > > + end =3D ti->len; > > + (void) sector_div(end, pool->sectors_per_block); > > + } > > + > > + offset -=3D ti->begin << SECTOR_SHIFT; > > + > > + while (true) { > > + begin =3D loff_to_block(pool, offset); > > + ret =3D dm_thin_find_mapped_range(td, begin, end, > > + &mapped_begin, &mapped_end, > > + &pool_begin, &maybe_shared); > > + if (ret =3D=3D -ENODATA) { > > + if (whence =3D=3D SEEK_DATA) > > + return -ENXIO; > > + break; > > + } else if (ret < 0) { > > + /* TODO handle EWOULDBLOCK? */ > > + return -ENXIO; >=20 > This should probably be -EIO, not -ENXIO. Yes. XFS also returns -EIO, so I guess it's okay to do so. I still need to get to the bottom of whether calling dm_thin_find_mapped_range() is sane here and what to do when/if it returns EWOULDBLOCK. > > + } > > + > > + /* SEEK_DATA finishes here... */ > > + if (whence =3D=3D SEEK_DATA) { > > + if (mapped_begin !=3D begin) > > + offset =3D block_to_loff(pool, mapped_begin); > > + break; > > + } > > + > > + /* ...while SEEK_HOLE may need to look further */ > > + if (mapped_begin !=3D begin) > > + break; /* offset is in a hole */ > > + > > + offset =3D block_to_loff(pool, mapped_end); > > + } > > + > > + return offset + (ti->begin << SECTOR_SHIFT); >=20 > It's hard to follow, but I'm fairly certain that if whence =3D=3D > SEEK_HOLE, you end up returning ti->begin + ti->len instead of -ENXIO > if the range from begin to end is fully mapped; which is inconsistent > with the semantics you have in 4/9 (although in 6/9 I argue that > having all of the dm callbacks return ti->begin + ti->len instead of > -ENXIO might make logic easier for iterating through consecutive ti, > and then convert to -ENXIO only in the caller). Returning (ti->begin + ti->len) << SECTOR_SHIFT for SEEK_HOLE when there is data at the end of the target is intentional. This matches the semantics of lseek(). I agree there is adjustment necessary in dm.c, but I want to seek the semantics of all lseek() functions identical to avoid confusion. Stefan --RRfDDrHk+pzHnTwS Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmYNb9IACgkQnKSrs4Gr c8iUVwf/Zpv3IsSznycexR7MwM8g1Pl1+FVxzDf79d7uGFplkj6L9AEhUrePfHRh cGxgeFgNqzKAAfDc6rWkEoGxm8Jq5J/OwaTQnMfS5qhmCSekIanhihEtmh2o+3vQ 5ugyOOADTaWx4AvxKNrfr+16kosIWS8NOHreUzqvYxEsqJ8H07PwZBDNUslQociS BtFimE8NmN40GYgH9KetDIyfzxwDE87WPWD61iQBKtsjpHaHcFcI1wsu+jqItN3i u7kLKlLBFyLGnuQjuCCYsOQR629RC1ZE4z2j2IEzJEOeQvnAD2YfGjBk7fdt1mXw bk76t2gtHwIgCVbdS5wnu6Bt+Wl6Ow== =zZmQ -----END PGP SIGNATURE----- --RRfDDrHk+pzHnTwS--