All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Matthew Wilcox <willy@infradead.org>
Cc: Christoph Hellwig <hch@lst.de>, linux-fsdevel@vger.kernel.org
Subject: Re: Uses of ->write_begin/write_end
Date: Tue, 16 Jul 2024 06:25:59 +0200	[thread overview]
Message-ID: <20240716042559.GA25209@lst.de> (raw)
In-Reply-To: <ZpVHaILAacPNlfyp@casper.infradead.org>

On Mon, Jul 15, 2024 at 04:59:36PM +0100, Matthew Wilcox wrote:
> I'm looking at ->write_begin() / ->write_end() again.  Here are our
> current callers:
> 
> drivers/gpu/drm/i915/gem/i915_gem_shmem.c:
> [1]	shmem_pwrite()
> [2]	i915_gem_object_create_shmem_from_data()

These really need to use actual shmem exported APIs, probably
shmem_get_folio, instead of abusing the aops.  With that we can
then easily kill ->write_begin() / ->write_end() for shmem.

> fs/affs/file.c:

Most of these fs-specific ones should really hardcode the calls to the
usually once or sometimes few potential instances that could be called
so that we can devirtualize the alls.

> fs/buffer.c:
> [4]	generic_cont_expand_simple()
> [5]	cont_expand_zero()
> [6]	cont_expand_zero()

> fs/namei.c:
> [B]	page_symlink()

> The copy_from_user() / memcpy() users feel like they should all end
> up calling ->write_iter().

> One way they could do this is by calling
> kernel_write() / __kernel_write(), but I'm not sure whether they
> should have the various accounting things (add_wchar(), inc_syscw())
> that happen inside __kernel_write_iter().
> 

They often sit much lower in the stack and/or are used for files that
don't have a ->write_iter.  e.g. page_symlink is obviously used for
symlinks that don't have ->write_iter.

For generic_cont_expand_simple goins through write_iter might be an
option, but instead of going through file ops the better idea might be
to just pass a write_iter-prototyped callback directly to it.

cont_expand_zero is a helper for cont_write_begin, which is used to
implement ->write_begin, so this actually already is a recursion, adding
another indirect to it is probably not helpful.


      parent reply	other threads:[~2024-07-16  4:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-15 15:59 Uses of ->write_begin/write_end Matthew Wilcox
2024-07-15 18:23 ` Matthew Wilcox
2024-07-16  4:25 ` Christoph Hellwig [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240716042559.GA25209@lst.de \
    --to=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.