Re: [PATCH 2/8] reiserfs: use kmap_local_folio() in _get_block_create_0()

On Sat 17-12-22 19:07:12, Matthew Wilcox wrote:
> On Sat, Dec 17, 2022 at 09:14:05AM -0800, Ira Weiny wrote:
> > On Fri, Dec 16, 2022 at 08:53:41PM +0000, Matthew Wilcox (Oracle) wrote:
> > > Switch from the deprecated kmap() to kmap_local_folio().  For the
> > > kunmap_local(), I subtract off 'chars' to prevent the possibility that
> > > p has wrapped into the next page.
> > 
> > Thanks for tackling this one.  I think the conversion is mostly safe because I
> > don't see any reason the mapping is passed to another thread.
> > 
> > But comments like this make me leary:
> > 
> >          "But, this means the item might move if kmap schedules"
> > 
> > What does that mean?  That seems to imply there is something wrong with the
> > base code separate from the kmapping.
> 
> I should probably have deleted that comment.  I'm pretty sure what it
> refers to is that we don't hold a lock that prevents the item from
> moving.  When ReiserFS was written, we didn't have CONFIG_PREEMPT, so 
> if kmap() scheduled, that was a point at which the item could move.
> I don't think I introduced any additional brokenness by converting
> from kmap() to kmap_local().  Maybe I'm wrong and somebody who
> understands ReiserFS can explain.

I think you've got it mostly right. Reiserfs comes from the times of the
Big Kernel Lock which reiserfs was using to protect all its data
structures. And that lock was automagically released when we scheduled. So
when kmap scheduled, the lock preventing item from moving got dropped and
we could have a problem. These days BKL in reiserfs got replaced by the
reiserfs_write_lock() / reiserfs_write_unlock() magic which mostly behaves
as a standard mutex so this is not a concern anymore.

									Honza
-- 
Jan Kara <[email protected]>
SUSE Labs, CR