On Thu, Feb 20, 2020 at 04:39:21PM -0800, Darrick J. Wong wrote:
> On Fri, Feb 21, 2020 at 11:01:19AM +1100, Dave Chinner wrote:
> > On Thu, Feb 20, 2020 at 10:34:50AM -0800, Darrick J. Wong wrote:
> > > > I think
> > > > having libxfs_umount() do a proper purge -> flush and returning any
> > > > errors instead is a fair tradeoff for simplicity. Removing the
> > > > flush_devices() API also eliminates risk of somebody incorrectly
> > > > attempting the flush after the umount frees the buftarg structures
> > > > (without reinitializing pointers :P).
> >
> > You mean like this code that I'm slowly working on to bring the
> > xfs_buf.c code across to userspace and get rid of the heap of crap
> > we have in libxfs/{rdwr,cache}.c now and be able to use AIO properly
> > and do non-synchronous delayed writes like we do in the kernel?
> >
> > libxfs/init.c:
> > ....
> > static void
> > buftarg_cleanup(
> > struct xfs_buftarg *btp)
> > {
> > if (!btp)
> > return;
> >
> > while (btp->bt_lru.l_count > 0)
> > xfs_buftarg_shrink(btp, 1000);
> > xfs_buftarg_wait(btp);
> > xfs_buftarg_free(btp);
>
> Not quite what the v3 series does, but only because it's still stuck
> with "whack the bcache and then go see what happened to each buftarg".
Right - I've completely reimplemented the caching and LRUs so that
global bcache thingy goes away.
> > }
> >
> > /*
> > * Release any resource obtained during a mount.
> > */
> > void
> > libxfs_umount(
> > struct xfs_mount *mp)
> > {
> > struct xfs_perag *pag;
> > int agno;
> >
> > libxfs_rtmount_destroy(mp);
> >
> > buftarg_cleanup(mp->m_ddev_targp);
> > buftarg_cleanup(mp->m_rtdev_targp);
> > if (mp->m_logdev_targp != mp->m_ddev_targp)
> > buftarg_cleanup(mp->m_logdev_targp);
>
> Yep, that's exactly where I moved the cleanup call in v3.
OK, good :P
> > .....
> >
> > libxfs/xfs_buftarg.c:
> > .....
> > void
> > xfs_buftarg_free(
> > struct xfs_buftarg *btp)
> > {
> > ASSERT(percpu_counter_sum(&btp->bt_io_count) == 0);
> > percpu_counter_destroy(&btp->bt_io_count);
> > platform_flush_device(btp->bt_fd, btp->bt_bdev);
> > libxfs_device_close(btp->bt_bdev);
> > free(btp);
>
> I'm assuming this means you've killed off the buffer handling parts of
> struct libxfs_xinit too?
Which bits are they? THe bcache init stuff is gone, yes, but right
now that function still does all the device opening and passing the
dev_ts to xfs_buftarg_alloc() for it to initialise similar to the
way the kernel initialises the buftarg.
> > I haven't added the error returns for this code yet - I'm still
> > doing the conversion and making it work.
> >
> > I'll probably have to throw the vast majority of that patchset away
> > and start again if all this API change that darrick has done is
> > merged. And that will probably involve me throwing away all of the
> > changes in this patch series because they just don't make any sense
> > once the code is restructured properly....
>
> ...or just throw them at me in whatever state they're in now and let me
> help figure out how to get there?
>
> Everyone: don't be afraid of the 'RFCRAP' for interim patchsets.
> Granted, posting git branches with a timestamp might be more
> practicable...
I'm not afraid of them - I've got to at least get it to the compile
stage with all the infrastructure in place and that's been the hold
up. :P
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
