Re: efivarfs and writev() support

From: Marcel Holtmann <marcel@holtmann.org>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Matt Fleming <matt@codeblueprint.co.uk>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-efi@vger.kernel.org,
	Matthew Garrett <matthew.garrett@nebula.com>,
	Jeremy Kerr <jk@ozlabs.org>,
	Matt Fleming <matt.fleming@intel.com>
Subject: Re: efivarfs and writev() support
Date: Sat, 14 Mar 2015 09:33:00 -0700	[thread overview]
Message-ID: <C20D75CF-FA82-43E5-BF70-E4E30E1D987B@holtmann.org> (raw)
In-Reply-To: <20150312170344.GN29656@ZenIV.linux.org.uk>

Hi Al,

>> I do not know about the specific semantics of efivarfs and frankly I have not tried every single combination. However it sounds to me that currently it requires that the whole file content is provided with a single write(). I have no idea if this is true or not. I do not know enough about the internals here.
>> 
>> Maybe efivarfs just needs to implemented .write_iter properly to actually support writev() and can not rely on a fallback of multiple write() calls.
> 
> Sigh...  There are three variants of write/writev semantics:
> 	1) stream.  Neither syscall nor vector member boundaries matter,
> the thing on the other end of IO channel might interpret the stream of
> data it's being fed and carve it into pieces, but that's a function of
> the contents.  TCP sockets are like that, so are pipes, etc.
> 	2) syscall-level datagram.  Vector member boundaries do not matter,
> syscall ones do.  UDP is like that - iovec is pure scatter-gather thing
> there; the boundaries come from syscalls.
> 	3) vector-level datagram.  Each vector member represents a single
> datagram, syscall boundaries do not matter.  I.e. iovec is an array of
> datagrams.  Most of character devices are like that.  And so's efivarfs.
> 
> What you are proposing seems to be switching it to syscall-level datagram
> behaviour.  It's very unlikely to break anything (I would be very surprised
> if anything tried to use "send this array of datagrams", simply because it's
> usually[1] bloody pointless for those files), but it *is* a user-visible API
> change.
> 
> And if we go for it, sure, we should just switch to ->write_iter() and be
> done with that - kmalloc(iov_iter_count(to), GFP_KERNEL), copy_from_iter(),
> use the first 4 bytes for attributes and the rest for body, same as we do
> now.

I say we should support writev() on efivarfs. Not supporting it seems odd especially since that is not documented anywhere. So yes, I am for adding .write_iter() support and be done with that.

Also please note that even write(,4) and write(,n) does not work either. You can not write partial entries as it seems. Maybe you are able to append, but it seems the initial creation of the variable has to be done with a single write() call. Anything else ends up in a file with 0 length.

Regards

Marcel