On Mon, 2020-12-14 at 15:34 +0000, Luis Henriques wrote:
> Jeff Layton <jlayton@xxxxxxxxxx> writes:
>
> > Add a generic function for taking an inode reference, setting the I_WORK
> > bit and queueing i_work. Turn the ceph_queue_* functions into static
> > inline wrappers that pass in the right bit.
> >
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > fs/ceph/inode.c | 55 ++++++-------------------------------------------
> > fs/ceph/super.h | 21 ++++++++++++++++---
> > 2 files changed, 24 insertions(+), 52 deletions(-)
> >
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index c870be90d850..9cd8b37e586a 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -1816,60 +1816,17 @@ void ceph_async_iput(struct inode *inode)
> > }
> > }
> >
> >
> > -/*
> > - * Write back inode data in a worker thread. (This can't be done
> > - * in the message handler context.)
> > - */
> > -void ceph_queue_writeback(struct inode *inode)
> > -{
> > - struct ceph_inode_info *ci = ceph_inode(inode);
> > - set_bit(CEPH_I_WORK_WRITEBACK, &ci->i_work_mask);
> > -
> > - ihold(inode);
> > - if (queue_work(ceph_inode_to_client(inode)->inode_wq,
> > - &ci->i_work)) {
> > - dout("ceph_queue_writeback %p\n", inode);
> > - } else {
> > - dout("ceph_queue_writeback %p already queued, mask=%lx\n",
> > - inode, ci->i_work_mask);
> > - iput(inode);
> > - }
> > -}
> > -
> > -/*
> > - * queue an async invalidation
> > - */
> > -void ceph_queue_invalidate(struct inode *inode)
> > -{
> > - struct ceph_inode_info *ci = ceph_inode(inode);
> > - set_bit(CEPH_I_WORK_INVALIDATE_PAGES, &ci->i_work_mask);
> > -
> > - ihold(inode);
> > - if (queue_work(ceph_inode_to_client(inode)->inode_wq,
> > - &ceph_inode(inode)->i_work)) {
> > - dout("ceph_queue_invalidate %p\n", inode);
> > - } else {
> > - dout("ceph_queue_invalidate %p already queued, mask=%lx\n",
> > - inode, ci->i_work_mask);
> > - iput(inode);
> > - }
> > -}
> > -
> > -/*
> > - * Queue an async vmtruncate. If we fail to queue work, we will handle
> > - * the truncation the next time we call __ceph_do_pending_vmtruncate.
> > - */
> > -void ceph_queue_vmtruncate(struct inode *inode)
> > +void ceph_queue_inode_work(struct inode *inode, int work_bit)
> > {
> > + struct ceph_fs_client *fsc = ceph_inode_to_client(inode);
> > struct ceph_inode_info *ci = ceph_inode(inode);
> > - set_bit(CEPH_I_WORK_VMTRUNCATE, &ci->i_work_mask);
> > + set_bit(work_bit, &ci->i_work_mask);
> >
> >
> > ihold(inode);
> > - if (queue_work(ceph_inode_to_client(inode)->inode_wq,
> > - &ci->i_work)) {
> > - dout("ceph_queue_vmtruncate %p\n", inode);
> > + if (queue_work(fsc->inode_wq, &ceph_inode(inode)->i_work)) {
>
> Nit: since we have ci, it should probably be used here^^ instead of
> ceph_inode() (this is likely a ceph_queue_invalidate function leftover,
> which already had this inconsistency).
>
> Other than that, these patches look good although I (obviously) haven't
> seen the lockdep warning you mention. Hopefully I'll never see it ever,
> with these patches applied ;-)
>
> Cheers,
Thanks Luis,
I went ahead and made the above change and pushed the set into the
testing branch (note that this patch supersedes the queue_inode_work
patch I sent a couple of weeks ago).
--
Jeff Layton <jlayton@xxxxxxxxxx>
