On 8/14/24 01:09, Olivier Langlois wrote:
On Tue, 2024-08-13 at 12:35 -0600, Jens Axboe wrote:
On 8/13/24 11:22 AM, Olivier Langlois wrote:
On Mon, 2024-08-12 at 14:40 -0600, Jens Axboe wrote:
3. I am surprised to notice that in __io_napi_do_busy_loop(),
list_for_each_entry_rcu() is called to traverse the list but
the
regular methods list_del() and list_add_tail() are called to
update
the
list instead of their RCU variant.
Should all just use rcu variants.
Here's a mashup of the changes. Would be great if you can test -
I'll
do
some too, but always good with more than one person testing as it
tends
to hit more cases.
Jens,
I have integrated our RCU corrections into
https://lore.kernel.org/io-uring/5fc9dd07e48a7178f547ed1b2aaa0814607fa246.1723567469.git.olivier@xxxxxxxxxxxxxx/T/#u
and my testing so far is not showing any problems...
but I have a very static setup...
I had no issues too without the corrections...
Thanks for testing, but regardless of whether that series would go in
or
not, I think those rcu changes should be done separately and upfront
rather than be integrated with other changes.
sorry about that...
I am going to share a little bit how I currently feel. I feel
disappointed because when I reread your initial reply, I have not been
able to spot a single positive thing said about my proposal despite
that I have prealably tested the water concerning my idea and the big
lines about how I was planning to design it. All, I have been told from
Pavel that the idea was so great that he was even currently playing
with a prototype around the same concept:
https://lore.kernel.org/io-uring/1be64672f22be44fbe1540053427d978c0224dfc.camel@xxxxxxxxxxxxxx/T/#mc7271764641f9c810ea5438ed3dc0662fbc08cb6
I've been playing with it but more of as a mean to some
other ideas. I had hard time to justify to myself to send
anything just to change the scheme, but it doesn't mean it's
a bad idea to get something like that merged. It just needs
some brushing mostly around excessive complexity, and it's
a part of normal dev process.
you also have to understand that all the small napi issues that I have
fixed this week are no stranger from me working on this new idea. The
RCU issues that I have reported back have been spotted when I was doing
my final code review before testing my patch before submitting it.
keep in mind that I am by far a git magician. I am a very casual
user... Anything that is outside the usual beaten trails such as
reordoring commits or breaking them down feels perilious to me...
I had 230+ lines changes committed when you confirmed that few lines
should be changed to address this new RCU issue. I did figure that it
would not that big a deal to include them with the rest of my change.
The main reason to have fixes in separate commits from new
features is because we're backporting fixes to older kernels.
It's usually just a cherry-pick, but being a part of a larger
commit complicates things a lot. There are also other usual
reasons like patch readability, keeping git history, not
mixing unrelated things together and so on.
--
Pavel Begunkov
