On Wed, Mar 15, 2023 at 6:26 PM Yu Kuai <yukuai1@xxxxxxxxxxxxxxx> wrote: > > Hi, > > 在 2023/03/16 6:55, Logan Gunthorpe 写道: [...] > > I was going to try and confirm that no new regressions were introduced > > by Yu's patches, but seems the tests are getting worse. I tried running > > the tests on the current md-next branch and found that one of the early > > tests, 00raid5-zero, hangs indefinitely. I quickly ran the same test on I am not able to repro the issue with 00raid5-zero. (I did a rebase before running the test, so that might be the reason). > > v6.3-rc2 and found that it runs just fine there. So it looks like > > there's already a regression in md-next that is not part of this series > > and I don't have the time to dig into the root cause right now. > > > > Yu's patches don't apply cleanly to v6.3-rc2 and I can't run the tests > > against md-next; so I didn't bother running them, but I did do a quick > > review. The locking changes make sense to me so it might be worth > > merging for correctness. However, I'm not entirely sure it's the best > > solution -- the md thread stuff seems like a bit of a mess and passing > > an mddev to thread functions that were not related to the mddev to get a > > lock seems to just make the mess a bit worse. > > > > For example, it seems a bit ugly to me for the lock mddev->thread_lock > > to protect the access of a pointer in struct r5l_log. Just spit-balling, > > but perhaps RCU would be more appropriate here. Then md_wakeup_thread() > > would just need to hold the RCU read lock when dereferencing, and > > md_unregister_thread() would just need to synchronize_rcu() before > > stopping and freeing the thread. This has the benefit of not requiring > > the mddev object for every md_thread and would probably require a lot > > less churn than the current patches. > > Thanks for your suggestion, this make sense to me. I'll try to use rcu. Yu Kuai, do you plan to resend the set with Logan suggestions? Thanks, Song
