All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Uladzislau Rezki <urezki@gmail.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Ext4 Developers List <linux-ext4@vger.kernel.org>,
	Suraj Jitindar Singh <surajjs@amazon.com>
Subject: Re: [PATCH RFC] ext4: fix potential race between online resizing and write operations
Date: Mon, 17 Feb 2020 14:33:14 -0500	[thread overview]
Message-ID: <20200217193314.GA12604@mit.edu> (raw)
In-Reply-To: <20200217160827.GA5685@pc636>

On Mon, Feb 17, 2020 at 05:08:27PM +0100, Uladzislau Rezki wrote:
> Hello, Joel, Paul, Ted. 
> 
> > 
> > Good point!
> > 
> > Now that kfree_rcu() is on its way to being less of a hack deeply
> > entangled into the bowels of RCU, this might be fairly easy to implement.
> > It might well be simply a matter of a function pointer and a kvfree_rcu()
> > API.  Adding Uladzislau Rezki and Joel Fernandez on CC for their thoughts.
> > 
> I think it makes sense. For example i see there is a similar demand in
> the mm/list_lru.c too. As for implementation, it will not be hard, i think. 
> 
> The easiest way is to inject kvfree() support directly into existing kfree_call_rcu()
> logic(probably will need to rename that function), i.e. to free vmalloc() allocations
> only in "emergency path" by just calling kvfree(). So that function in its turn will
> figure out if it is _vmalloc_ address or not and trigger corresponding "free" path.

The other difference between ext4_kvfree_array_rcu() and kfree_rcu()
is that kfree_rcu() is a magic macro which frees a structure, which
has to contain a struct rcu_head.  In this case, I'm freeing a pointer
to set of structures, or in another case, a set of buffer_heads, which
do *not* have an rcu_head structure.

> struct test_kvfree_rcu {
>        unsigned char array[PAGE_SIZE * 2];
>        struct rcu_head rcu;
> };

I suspect I'd still want to use the ext4_kfree_array_rcu(), for a
couple of reasons.  First of all, the array is variably sized.  So we
don't know how big it is.  That could be fixed via something like 

struct test_kvfree_rcu {
       struct rcu_head rcu;
       struct test_s [];
};

... but the other issue is that we have code where we have arrays of
arrays, e.g.:

	struct ext4_group_info ***s_group_info;

which is an array of array of pointers to ext4_group_info.  Trying to
cram in the rcu_head makes the code more complicated --- and also,
resizing file systems is something that happens often, and I don't
want to optimize it by keeping rcu_head structs around all the time.

This is why at least for *this* use case, it's actually better to
allocate temp array just before callig call_rcu(), and if I can't
allocate it due to memory pressure, we'll it's OK to use
synchronize_rcu() followed by kvfree().

Cheers,

						- Ted

  reply	other threads:[~2020-02-17 19:33 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-15 23:38 [PATCH RFC] ext4: fix potential race between online resizing and write operations Theodore Y. Ts'o
2020-02-16 12:12 ` Paul E. McKenney
2020-02-16 20:32   ` Theodore Y. Ts'o
2020-02-17 16:08   ` Uladzislau Rezki
2020-02-17 19:33     ` Theodore Y. Ts'o [this message]
2020-02-18 17:08       ` Uladzislau Rezki
2020-02-20  4:52         ` Theodore Y. Ts'o
2020-02-21  0:30           ` Paul E. McKenney
2020-02-21 13:14             ` Uladzislau Rezki
2020-02-21 20:22               ` Paul E. McKenney
2020-02-22 22:24                 ` Joel Fernandes
2020-02-23  1:10                   ` Paul E. McKenney
2020-02-24 17:40                     ` Uladzislau Rezki
2020-02-25  2:07                       ` Joel Fernandes
2020-02-25  3:55                         ` Paul E. McKenney
2020-02-25 14:17                           ` Joel Fernandes
2020-02-25 16:38                             ` Paul E. McKenney
2020-02-25 17:00                               ` Joel Fernandes
2020-02-25 18:54                         ` Uladzislau Rezki
2020-02-25 22:47                           ` Paul E. McKenney
2020-02-26 13:04                             ` Uladzislau Rezki
2020-02-26 15:06                               ` Paul E. McKenney
2020-02-26 15:53                                 ` Uladzislau Rezki
2020-02-27 14:08                                   ` Joel Fernandes
2020-03-01 11:13                                     ` Uladzislau Rezki
2020-02-27 13:37                           ` Joel Fernandes
2020-03-01 11:08                             ` Uladzislau Rezki
2020-03-01 12:07                               ` Uladzislau Rezki
2020-02-25  2:11                     ` Joel Fernandes
2020-02-21 12:06         ` Joel Fernandes
2020-02-21 13:28           ` Joel Fernandes
2020-02-21 19:21             ` Uladzislau Rezki
2020-02-21 19:25               ` Uladzislau Rezki
2020-02-22 22:12               ` Joel Fernandes
2020-02-24 17:02                 ` Uladzislau Rezki
2020-02-24 23:14                   ` Paul E. McKenney
2020-02-25  1:48                   ` Joel Fernandes
2020-02-19  3:09 ` Jitindar SIngh, Suraj
2020-02-20  4:34   ` Theodore Y. Ts'o

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=20200217193314.GA12604@mit.edu \
    --to=tytso@mit.edu \
    --cc=joel@joelfernandes.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=surajjs@amazon.com \
    --cc=urezki@gmail.com \
    /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.