Hello Jason, Thanks for keeping me in the loop on this. I've also added the locking maintainers in Cc. IMHO there are some seqlock.h API violations in this series, and they should have the final say on this. On Fri, Oct 30, 2020 at 11:46:19AM -0300, Jason Gunthorpe wrote: > > As discussed and suggested by Linus use a seqcount to close the small race > between gup_fast and copy_page_range(). > > Unfortunately the good suggestion to just use write_seqcount_begin() blows > up lockdep immediately due to the (new?) requirement that the write side > of seqcount be in a preempt disabled region. Disabling preemption for seqcount_t write-side critical sections was never a new requirement. It has always been this way, for the reasons explained at Documentation/locking/seqlock.rst, "Introduction" section. The recent seqcount_t changes did not mandate any new rules. This was done explicitly, and on-purpose, not to break any of the *large* set of existing seqcount_t call sites. It added multiple lockdep asserts though, to catch a number of (already) buggy users, and they were fixed beforehand. It seems you have a special case here, so I'll continue discussing this at patch #2 where the code resides. Just wanted to answer the "(new?)" part above. > For this application it does > not seem like a good idea, nor is it necessary as we don't spin on retry. > This is solved by being the first place to use raw_write_seqcount_t_begin() > Regardless of this series write side preemptibility requirements, the "_write_seqcount_*t*_()" interfaces are internal to seqlock.h and should _never_ be used outside of it. For plain seqcount_t, raw_write_seqcount_begin() is equivalent to raw_write_seqcount_*t*_begin() anyway, and should already satisfy your needs. /me jumps to patch #2 now... Thanks, -- Ahmed S. Darwish Linutronix GmbH
