From patchwork Sun Aug 19 21:22:24 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sage Weil X-Patchwork-Id: 1345331 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 6A168DF264 for ; Sun, 19 Aug 2012 21:22:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751310Ab2HSVWZ (ORCPT ); Sun, 19 Aug 2012 17:22:25 -0400 Received: from cobra.newdream.net ([66.33.216.30]:58793 "EHLO cobra.newdream.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751127Ab2HSVWY (ORCPT ); Sun, 19 Aug 2012 17:22:24 -0400 Received: from cobra.newdream.net (localhost [127.0.0.1]) by cobra.newdream.net (Postfix) with ESMTP id 82BD280454; Sun, 19 Aug 2012 14:22:24 -0700 (PDT) Received: by cobra.newdream.net (Postfix, from userid 1031) id 6536280486; Sun, 19 Aug 2012 14:22:24 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by cobra.newdream.net (Postfix) with ESMTP id 5136F80454; Sun, 19 Aug 2012 14:22:24 -0700 (PDT) Date: Sun, 19 Aug 2012 14:22:24 -0700 (PDT) From: Sage Weil X-X-Sender: sage@cobra.newdream.net To: Alexandre Oliva cc: Sage Weil , ceph-devel@vger.kernel.org Subject: Re: [PATCH] Add old_inodes to emetablob In-Reply-To: Message-ID: References: User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org On Sun, 19 Aug 2012, Sage Weil wrote: > On Sun, 19 Aug 2012, Alexandre Oliva wrote: > > On Aug 18, 2012, Alexandre Oliva wrote: > > > > > I've looked further into this tonight, and I found out what modifies > > > the timestamps in the inode is (re)issuing caps to a client. > > > > > So, how can we stop pre_cow_old_inode from messing with the old_inode? > > > Any suggestions? > > > > This patch seems to avoid the problem, but is it correct, or is it just > > papering over a problem elsewhere? > > I think the real bug is that CInode::first is less than one of the > old_inodes keys. Looking at cow_old_inode(), it looks like first should > always be strictly > than all old_inodes keys. We should probably assert > as much wherever it is convenient to do so... > > In any case, can you see where/how that is happening? If this is a dir > inode that has been commited and then refetched from the parent dir, that > probably explains it. For non-multiversion inodes, dn->first == > in->first, but not so for these guys, and at first glance I'm not seeing > it in the CDir::_fetched() code. This might do the trick? > > sage > > > > > Subject: mds: Don't modify already-created old_inode > > > > In cow_old_inode, do not modify an old_inode that was created before. > > > > Signed-off-by: Alexandre Oliva > > --- > > src/mds/CInode.cc | 14 +++++++++----- > > 1 files changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc > > index 53f9e69..fdff86c 100644 > > --- a/src/mds/CInode.cc > > +++ b/src/mds/CInode.cc > > @@ -2030,12 +2030,16 @@ old_inode_t& CInode::cow_old_inode(snapid_t follows, bool cow_head) > > inode_t *pi = cow_head ? get_projected_inode() : get_previous_projected_inode(); > > map *px = cow_head ? get_projected_xattrs() : get_previous_projected_xattrs(); > > > > + bool found = old_inodes.find(follows) != old_inodes.end(); > > old_inode_t &old = old_inodes[follows]; > > - old.first = first; > > - old.inode = *pi; > > - old.xattrs = *px; > > - > > - dout(10) << " " << px->size() << " xattrs cowed, " << *px << dendl; > > + > > + if (!found) { > > + old.first = first; > > + old.inode = *pi; > > + old.xattrs = *px; > > + > > + dout(10) << " " << px->size() << " xattrs cowed, " << *px << dendl; > > + } > > > > old.inode.trim_client_ranges(follows); > > > > -- > > 1.7.7.6 > > > > > > > > -- > > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > > You must be the change you wish to see in the world. -- Gandhi > > Be Free! -- http://FSFLA.org/ FSF Latin America board member > > Free Software Evangelist Red Hat Brazil Compiler Engineer > > -- > > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > > -- > To unsubscribe from this list: send the line "unsubscribe ceph-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > --- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc index 6761c6f..9034978 100644 --- a/src/mds/CDir.cc +++ b/src/mds/CDir.cc @@ -1564,6 +1564,8 @@ void CDir::_fetched(bufferlist &bl, const string& want_dn) in->xattrs.swap(xattrs); in->decode_snap_blob(snapbl); in->old_inodes.swap(old_inodes); + if (!in->old_inodes.empty()) + in->first = in->old_inodes.rbegin()->first + 1; if (snaps) in->purge_stale_snap_data(*snaps);