mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: NeilBrown <neilb@suse.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kernfs: use stack-buf for small writes.
Date: Tue, 23 Sep 2014 00:55:49 -0400	[thread overview]
Message-ID: <20140923045549.GB11740@mtj.dyndns.org> (raw)
In-Reply-To: <20140923144650.2aa71b39@notabene.brown>

Hello, Neil.

On Tue, Sep 23, 2014 at 02:46:50PM +1000, NeilBrown wrote:
> seqfile is only safe for reads.  sysfs via kernfs uses seq_read(), so there
> is only a single allocation on the first read.
> 
> It doesn't really related to fixing writes, except to point out that only
> writes need to be "fixed".  Reads already work.

Oh, I meant the buffer seqfile read op writes to, so it depends on the
fact that the allocation is only on the first read?  That seems
extremely brittle to me, especially for an issue which tends to be
difficult to reproduce.

> Separately:
> 
> > Ugh... :( If this can't be avoided at all, I'd much prefer it to be
> > something explicit - a flag marking the file as needing a persistent
> > write buffer which is allocated on open.  "Small" writes on stack
> > feels way to implicit to me.
> 
> How about if we add seq_getbuf() and seq_putbuf() to seqfile
> which takes a 'struct seq_file' and a size and returns the ->buf
> after making sure it is big enough.
> It also claims and releases the seqfile ->lock.
> 
> Then we would be using the same buffer for reads and write.
> 
> Does that sound suitable?  It uses existing infrastructure and avoids having
> to identify in advance which attributes it is important for.

I'd much rather keep things direct and make it explicitly allocate r/w
buffer(s) on open and disallow seq_file operations on such files.

Thanks.

-- 
tejun

  reply	other threads:[~2014-09-23  4:55 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23  4:06 NeilBrown
2014-09-23  4:12 ` Tejun Heo
2014-09-23  4:18 ` Tejun Heo
2014-09-23  4:46   ` NeilBrown
2014-09-23  4:55     ` Tejun Heo [this message]
2014-09-23  5:40       ` NeilBrown
2014-09-23  5:51         ` Tejun Heo
2014-09-23  6:11           ` NeilBrown
2014-09-23  6:15             ` Tejun Heo
2014-09-24 19:17 ` Al Viro
2014-09-24 23:58   ` NeilBrown

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=20140923045549.GB11740@mtj.dyndns.org \
    --to=tj@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®