Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: XUtWyBJNY3Qbg80WUIO7J60qRjjlTJhqACzG3qvGYVlQXZ7LEsbenmHqh/63lL8BKVlJF1gSn7
 CM5qUkrQthOOBmd+cYVi21mr4Q0liZIEzkvb0P84/wZ7bfARRmdF/XdEp2fNWnuPMHge3iNWLA
 v5phXqPCkIqEXeRFLVnZCpM3OI2PpGyjh5pjZgmyXYGLm9xfgo7zd15pOCoJAjlOM597mzKCun
 4pzJ12hRMMz899LXJhMhTiWyrwJb55EqIYqJeS1sWOs/TrAlyg3O3ypre9uC4r1Nl8UH4iGJzW
 USs=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     "hch@infradead.org" <hch@infradead.org>
CC:     Kanchan Joshi <joshiiitr@gmail.com>, Jens Axboe <axboe@kernel.dk>,
        Pavel Begunkov <asml.silence@gmail.com>,
        Kanchan Joshi <joshi.k@samsung.com>,
        "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
        "bcrl@kvack.org" <bcrl@kvack.org>,
        Matthew Wilcox <willy@infradead.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-aio@kvack.org" <linux-aio@kvack.org>,
        "io-uring@vger.kernel.org" <io-uring@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
        SelvaKumar S <selvakuma.s1@samsung.com>,
        Nitesh Shetty <nj.shetty@samsung.com>,
        Javier Gonzalez <javier.gonz@samsung.com>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        Naohiro Aota <Naohiro.Aota@wdc.com>
Subject: Re: [PATCH v4 6/6] io_uring: add support for zone-append
Thread-Topic: [PATCH v4 6/6] io_uring: add support for zone-append
Thread-Index: AQHWYdbcc0q15qREKECGO7brHi6zEg==
Date:   Fri, 31 Jul 2020 10:16:49 +0000
Message-ID: <MWHPR04MB3758A4B2967DB1FABAAD9265E74E0@MWHPR04MB3758.namprd04.prod.outlook.com>
References: <CA+1E3rJ3SoLU9aYcugAQgJnSPnJtcCwjZdMREXS3FTmXgy3yow@mail.gmail.com>
 <f030a338-cd52-2e83-e1da-bdbca910d49e@kernel.dk>
 <CA+1E3rKxZk2CatTuPcQq5d14vXL9_9LVb2_+AfR2m9xn2WTZdg@mail.gmail.com>
 <MWHPR04MB3758DC08EA17780E498E9EC0E74E0@MWHPR04MB3758.namprd04.prod.outlook.com>
 <20200731064526.GA25674@infradead.org>
 <MWHPR04MB37581344328A42EA7F5ED13EE74E0@MWHPR04MB3758.namprd04.prod.outlook.com>
 <CA+1E3rLM4G4SwzD6RWsK6Ssp7NmhiPedZDjrqN3kORQr9fxCtw@mail.gmail.com>
 <MWHPR04MB375863C20C1EF2CB27E62703E74E0@MWHPR04MB3758.namprd04.prod.outlook.com>
 <20200731091416.GA29634@infradead.org>
 <MWHPR04MB37586D39CA389296CE0252A4E74E0@MWHPR04MB3758.namprd04.prod.outlook.com>
 <20200731094135.GA4104@infradead.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 5410a628-1376-45fc-5312-08d8353ad6a2
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Jul 2020 10:16:49.7513
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: v8el+wJ+IbVLJ5V57YDRZXxWa5pGm8yEdr+IHhNaQ8LxKpRKPwEl01npf3WKj3t/eE7ItufmvZzwFvJGy2oGuw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR04MB1039
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/07/31 18:41, hch@infradead.org wrote:=0A=
> On Fri, Jul 31, 2020 at 09:34:50AM +0000, Damien Le Moal wrote:=0A=
>> Sync writes are done under the inode lock, so there cannot be other writ=
ers at=0A=
>> the same time. And for the sync case, since the actual written offset is=
=0A=
>> necessarily equal to the file size before the write, there is no need to=
 report=0A=
>> it (there is no system call that can report that anyway). For this sync =
case,=0A=
>> the only change that the use of zone append introduces compared to regul=
ar=0A=
>> writes is the potential for more short writes.=0A=
>>=0A=
>> Adding a flag for "report the actual offset for appending writes" is fin=
e with=0A=
>> me, but do you also mean to use this flag for driving zone append write =
vs=0A=
>> regular writes in zonefs ?=0A=
> =0A=
> Let's keep semantics and implementation separate.  For the case=0A=
> where we report the actual offset we need a size imitation and no=0A=
> short writes.=0A=
=0A=
OK. So the name of the flag confused me. The flag name should reflect "Do z=
one=0A=
append and report written offset", right ?=0A=
=0A=
Just to clarify, here was my thinking for zonefs:=0A=
1) file open with O_APPEND/aio has RWF_APPEND: then it is OK to assume that=
 the=0A=
application did not set the aio offset since APPEND means offset=3D=3Dfile =
size. In=0A=
that case, do zone append and report back the written offset.=0A=
2) file open without O_APPEND/aio does not have RWF_APPEND: the application=
=0A=
specified an aio offset and we must respect it, write it that exact same or=
der,=0A=
so use regular writes.=0A=
=0A=
For regular file systems, with case (1) condition, the FS use whatever it w=
ants=0A=
for the implementation, and report back the written offset, which  will alw=
ays=0A=
be the file size at the time the aio was issued.=0A=
=0A=
Your method with a new flag to switch between (1) and (2) is OK with me, bu=
t the=0A=
"no short writes" may be difficult to achieve in a regular FS, no ? I do no=
t=0A=
think current FSes have such guarantees... Especially in the case of buffer=
ed=0A=
async writes I think.=0A=
=0A=
> Anything with those semantics can be implemented using Zone Append=0A=
> trivially in zonefs, and we don't even need the exclusive lock in that=0A=
> case.  But even without that flag anything that has an exclusive lock can=
=0A=
> at least in theory be implemented using Zone Append, it just need=0A=
> support for submitting another request from the I/O completion handler=0A=
> of the first.  I just don't think it is worth it - with the exclusive=0A=
> lock we do have access to the zone serialied so a normal write works=0A=
> just fine.  Both for the sync and async case.=0A=
=0A=
We did switch to have zonefs do append writes in the sync case, always. Hmm=
m...=0A=
Not sure anymore it was such a good idea.=0A=
=0A=
> =0A=
>> The fcntl or ioctl for getting the max atomic write size would be fine t=
oo.=0A=
>> Given that zonefs is very close to the underlying zoned drive, I was ass=
uming=0A=
>> that the application can simply consult the device sysfs zone_append_max=
_bytes=0A=
>> queue attribute.=0A=
> =0A=
> For zonefs we can, yes.  But in many ways that is a lot more cumbersome=
=0A=
> that having an API that works on the fd you want to write on.=0A=
=0A=
Got it. Makes sense.=0A=
=0A=
>> For regular file systems, this value would be used internally=0A=
>> only. I do not really see how it can be useful to applications. Furtherm=
ore, the=0A=
>> file system may have a hard time giving that information to the applicat=
ion=0A=
>> depending on its underlying storage configuration (e.g. erasure=0A=
>> coding/declustered RAID).=0A=
> =0A=
> File systems might have all kinds of limits of their own (e.g. extent=0A=
> sizes).  And a good API that just works everywhere and is properly=0A=
> documented is much better than heaps of cargo culted crap all over=0A=
> applications.=0A=
=0A=
OK. Makes sense. That said, taking Naohiro's work on btrfs as an example, z=
one=0A=
append is used for every data write, no matter if it is O_APPEND/RWF_APPEND=
 or=0A=
not. The size limitation for zone append writes is not needed at all by=0A=
applications. Maximum extent size is aligned to the max append write size=
=0A=
internally, and if the application issued a larger write, it loops over mul=
tiple=0A=
extents, similarly to any regular write may do (if there is overwrite etc..=
.).=0A=
=0A=
For the regular FS case, my thinking on the semantic really was: if asked t=
o do=0A=
so, return the written offset for a RWF_APPEND aios. And I think that=0A=
implementing that does not depend in any way on what the FS does internally=
.=0A=
=0A=
But I think I am starting to see the picture you are drawing here:=0A=
1) Introduce a fcntl() to get "maximum size for atomic append writes"=0A=
2) Introduce an aio flag specifying "Do atomic append write and report writ=
ten=0A=
offset"=0A=
3) For an aio specifying "Do atomic append write and report written offset"=
, if=0A=
the aio is larger than "maximum size for atomic append writes", fail it on=
=0A=
submission, no short writes.=0A=
4) For any other aio, it is business as usual, aio is processed as they are=
 now.=0A=
=0A=
And the implementation is actually completely free to use zone append write=
s or=0A=
regular writes regardless of the "Do atomic append write and report written=
=0A=
offset" being used or not.=0A=
=0A=
Is it your thinking ? That would work for me. That actually end up complete=
ly=0A=
unifying the interface behavior for zonefs and regular FS. Same semantic.=
=0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=