linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Boaz Harrosh <boaz@plexistor.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH vfs 2/2] {block|char}_dev: remove inode->i_devices
Date: Thu, 20 Nov 2014 06:50:52 -0500	[thread overview]
Message-ID: <20141120115052.GA32237@htj.dyndns.org> (raw)
In-Reply-To: <546DC5AD.3040606@plexistor.com>

Hello, Boaz.

On Thu, Nov 20, 2014 at 12:42:53PM +0200, Boaz Harrosh wrote:
> if I understand correctly the motivation here is that the allocation
> of the internal element is done GFP_KERNEL at this call
> 
> Then the add() below can be under the spin_lock.
> 
> So why don't you just return an element here to caller and give it to
> add below. No Preemption-disable, no percpu variable, simple. Like:

Hmmm... mostly because preloading is more convenient and but also
because it provides better separation from internal implementation
details.  e.g. This may be implemented using a different data
structure (e.g. bonsai tree) which may require differing number of new
elements even on success.  With the scheme you're describing, the
operation would be constantly allocating and freeing memory areas
(which may be multiple) unnecessarily.

One thing which is debatable is how to handle preloading errors.  We
can have the preload fail and then assume that the later insertion
won't fail with -ENOMEM (often through BUG/WARN_ON()); however, it
often, but not always, is that those insertion operations may fail
with different error codes too and requires error handling anyway, so
overall it seems better to defer the allocation error to the actual
insertion point.  It also makes conceptual sense.  The preloading
simply upgrades the allocation mask the insertion operation uses.

Thanks.

-- 
tejun

  reply	other threads:[~2014-11-20 11:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-13 22:09 [PATCH vfs 1/2] lib: implement ptrset Tejun Heo
2014-11-13 22:11 ` [PATCH vfs 2/2] {block|char}_dev: remove inode->i_devices Tejun Heo
2014-11-18 12:10   ` Boaz Harrosh
2014-11-18 12:30     ` Tejun Heo
2014-11-20 10:42   ` Boaz Harrosh
2014-11-20 11:50     ` Tejun Heo [this message]
2014-11-20 12:36       ` Boaz Harrosh
2014-11-20 13:11         ` Tejun Heo
2014-11-20 13:39           ` Tejun Heo
2014-11-20 14:14           ` Boaz Harrosh
2014-11-20 14:19             ` Tejun Heo
2014-11-13 22:23 ` [PATCH vfs 1/2] lib: implement ptrset Andrew Morton
2014-11-13 22:27   ` Tejun Heo
2014-11-13 22:40     ` Andrew Morton
2014-11-14 13:12       ` Tejun Heo
2014-11-18 20:46         ` Andrew Morton
2014-11-18  9:19 ` Lai Jiangshan
2014-11-18 11:55   ` Tejun Heo
2014-11-19  1:41     ` Lai Jiangshan
2014-11-18 15:56 ` Azat Khuzhin
2014-11-18 17:16   ` Tejun Heo
2014-11-18 17:49 ` [PATCH vfs v2 " Tejun Heo
2014-11-25 16:37 ` [PATCH vfs " Jan Kara
2014-12-02 18:15   ` Tejun Heo

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=20141120115052.GA32237@htj.dyndns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=boaz@plexistor.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).