From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oren Laadan Subject: Re: [PATCH 06/16][cr][v3]: Checkpoint file-locks Date: Wed, 04 Aug 2010 19:26:18 -0400 Message-ID: <4C59F71A.2010002@cs.columbia.edu> References: <1280877097-12377-1-git-send-email-sukadev@linux.vnet.ibm.com> <1280877097-12377-7-git-send-email-sukadev@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Serge Hallyn , Matt Helsley , Dan Smith , John Stultz , Matthew Wilcox , Jamie Lokier , linux-fsdevel@vger.kernel.org, Containers To: Sukadev Bhattiprolu Return-path: Received: from brinza.cc.columbia.edu ([128.59.29.8]:47428 "EHLO brinza.cc.columbia.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756162Ab0HDX0q (ORCPT ); Wed, 4 Aug 2010 19:26:46 -0400 In-Reply-To: <1280877097-12377-7-git-send-email-sukadev@linux.vnet.ibm.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 08/03/2010 07:11 PM, Sukadev Bhattiprolu wrote: > While checkpointing each file-descriptor, find all the locks on the > file and save information about the lock in the checkpoint-image. > A follow-on patch will use this informaiton to restore the file-locks. > > Changelog[v3]: > [Oren Laadan] Add a missing (loff_t) type cast and use a macro > to set the marker/dummy file lock > > Changelog[v2]: > [Matt Helsley]: Use fixed sizes (__s64) instead of 'loff_t' in > 'struct ckpt_hdr_file_lock'. > [Matt Helsley, Serge Hallyn]: Highlight new use of BKL (using > lock_flocks() macros as suggested by Serge). > [Matt Helsley]: Reorg code a bit to simplify error handling. > [Matt Helsley]: Reorg code to initialize marker-lock (Pass a > NULL lock to checkpoint_one_lock() to indicate marker). > > Signed-off-by: Sukadev Bhattiprolu > --- > fs/checkpoint.c | 100 ++++++++++++++++++++++++++++++++++----- > include/linux/checkpoint_hdr.h | 18 +++++++ > 2 files changed, 105 insertions(+), 13 deletions(-) > > diff --git a/fs/checkpoint.c b/fs/checkpoint.c > index b5486c1..57b6944 100644 > --- a/fs/checkpoint.c > +++ b/fs/checkpoint.c > @@ -26,8 +26,19 @@ > #include > #include > #include > +#include > #include > > +/* > + * TODO: This code uses the BKL for consistency with other uses of > + * 'for_each_lock()'. But since the BKL may be replaced with another > + * lock in the future, use lock_flocks() macros instead. lock_flocks() > + * are currently used in BKL-fix sand boxes and when those changes > + * are merged, the following macros can be removed > + */ > +#define lock_flocks() lock_kernel() > +#define unlock_flocks() unlock_kernel() > + > /************************************************************************** > * Checkpoint > */ > @@ -256,8 +267,78 @@ static int checkpoint_file(struct ckpt_ctx *ctx, void *ptr) > return ret; > } I prefer the approach of writing-all-at-once over the current one-at-a-time-and-mark-the-end. See my previous comment: (https://lists.linux-foundation.org/pipermail/containers/2010-July/025057.html) --- Having looked at the code again - how about the following: get rid of this "last entry" altogether; instead, during checkpoint, first count the locks, then write a header with the number of locks, following by that many records of the locks themselves. This is what we do for other resource lists/chains as well. Also makes it a bit easier to parse since you always know what to expect once you see the header. --- (Yes, I'm aware that if the list is long you may need to send multiple chunks and for that "remember" where you were in the list...) [...] Oren.