[{"id":1758066,"web_url":"http://patchwork.ozlabs.org/comment/1758066/","msgid":"<20170826153313.le7biqxzg7mz6f32@thunk.org>","list_archive_url":null,"date":"2017-08-26T15:33:14","subject":"Re: [PATCH] e2fsck: add optimization for heavily hard-linked file\n\tsystems","submitter":{"id":350,"url":"http://patchwork.ozlabs.org/api/people/350/","name":"Theodore Tso","email":"tytso@mit.edu"},"content":"On Thu, Aug 24, 2017 at 11:08:52AM +0200, Jaco Kroon wrote:\n> > +#define E2F_OPT_ICOUNT_FULLMAP\t0x20000 /* don't optimize extents */\n> description here seems wrong? Perhaps \"use fullmap for inode link count\" ?\n\nOops, fixed.\n\n> > --- a/e2fsck/pass1.c\n> > +++ b/e2fsck/pass1.c\n> > @@ -711,6 +711,7 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name,\n> > \terrcode_t\t\tretval;\n> > \tchar\t\t\t*tdb_dir;\n> > \tint\t\t\tenable;\n> > +\tint\t\t\tfull_map;\n> > \t*ret = 0;\n> > @@ -734,6 +735,8 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name,\n> > \t}\n> > \te2fsck_set_bitmap_type(ctx->fs, EXT2FS_BMAP64_RBTREE, icount_name,\n> > \t\t\t &save_type);\n> > +\tif (ctx->options & E2F_OPT_ICOUNT_FULLMAP)\n> > +\t\tflags |= EXT2_ICOUNT_OPT_FULLMAP;\n> > \tretval = ext2fs_create_icount2(ctx->fs, flags, 0, hint, ret);\n> > \tctx->fs->default_bitmap_type = save_type;\n> > \treturn retval;\n> This isn't used for pass1 (I did originally use full_map either way then\n> decided to only use it for pass2) - and we to receive flags coming into this\n> function - shouldn't this perhaps rather go into pass2.c (e2fsck_pass2\n> function to be more specific)? This really is cosmetic imho.\n\nThe e2fsck_setup_icount() function is used for both pass1 and pass2.\n\n> > diff --git a/e2fsck/unix.c b/e2fsck/unix.c\n> > index ff961483b..939862f1a 100644\n> > --- a/e2fsck/unix.c\n> > +++ b/e2fsck/unix.c\n> > @@ -709,9 +709,18 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)\n> > \t\t} else if (strcmp(token, \"nodiscard\") == 0) {\n> > \t\t\tctx->options &= ~E2F_OPT_DISCARD;\n> > \t\t\tcontinue;\n> > +\t\t} else if (strcmp(token, \"optimize_extents\") == 0) {\n> > +\t\t\tctx->options &= ~E2F_OPT_NOOPT_EXTENTS;\n> > +\t\t\tcontinue;\n> unrelated, but makes sense :).\n\nYes, this was a cleanup I was doing as I weant.\n\n> > +\t\tif (!retval) {\n> > +\t\t\tmemset(icount->fullmap, 0,\n> > +\t\t\t*sizeof(__u32)* * fs->super->s_inodes_count); <-------\n> this of course should be sizeof(__u16), or sizeof(*count->fullmap) which\n> will track the type pointed to. It might be better practice to store\n> \"sizeof(*icount->fullmap) * fs->super->s_inodes_count\" to a variable and\n> just re-use it.\n\nGood point. Fixed.\n\n> > \t\tgoto errout;\n> > +\n> Extra line can be removed.\n> > \tif (flags & EXT2_ICOUNT_OPT_INCREMENT) {\n\nRemoved.\n\nThanks for taking a look at the patch!\n\n\t\t\t\t\t\t- Ted","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-ext4-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=thunk.org header.i=@thunk.org\n\theader.b=\"cWIwITl3\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xfhrX74GGz9t5R\n\tfor ;\n\tSun, 27 Aug 2017 01:33:32 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751063AbdHZPdT (ORCPT );\n\tSat, 26 Aug 2017 11:33:19 -0400","from imap.thunk.org ([74.207.234.97]:57090 \"EHLO imap.thunk.org\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751010AbdHZPdS (ORCPT );\n\tSat, 26 Aug 2017 11:33:18 -0400","from root (helo=callcc.thunk.org)\n\tby imap.thunk.org with local-esmtp (Exim 4.84_2)\n\t(envelope-from )\n\tid 1dld5H-0000Rk-2C; Sat, 26 Aug 2017 15:33:15 +0000","by callcc.thunk.org (Postfix, from userid 15806)\n\tid 224F9C05EC9; Sat, 26 Aug 2017 11:33:14 -0400 (EDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=thunk.org;\n\ts=ef5046eb; \n\th=In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date;\n\tbh=x+Clcxc+oW3RIdsGOLb5OAWJ/4LVts2RSmQKpSAnEKI=; \n\tb=cWIwITl362ZzCnIPI6mxUe5s0HRHANjxyrv2TEfRn2dMyWRYTnwpMP2390rVVgeDFY6EGfYdF1JcQSstQclGzrabXh52K3P2m03DUavU/FBUAlpjBGtK/c1vkWq3ng+e1h4QVTrW/K5ZXiBJElwIWUmiIbq0ZVKRniNNgyprULQ=;","Date":"Sat, 26 Aug 2017 11:33:14 -0400","From":"Theodore Ts'o ","To":"Jaco Kroon ","Cc":"Doug Porter , linux-ext4@vger.kernel.org,\n\tOmar Sandoval ","Subject":"Re: [PATCH] e2fsck: add optimization for heavily hard-linked file\n\tsystems","Message-ID":"<20170826153313.le7biqxzg7mz6f32@thunk.org>","References":"<20170819011635.1815929-1-dsp@fb.com>\n\t<20170822022948.nyn6fessudjaj5xh@thunk.org>\n\t\n\t<20170822124505.xr7wnxonsbml3mgh@thunk.org>\n\t<00f408c1-215e-39e2-dec4-8f05eb604f97@uls.co.za>\n\t<20170823232351.643l4wg7pvnnxyts@thunk.org>\n\t<48660f5d-2e53-f0ff-e0e5-169a1d2d631f@uls.co.za>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<48660f5d-2e53-f0ff-e0e5-169a1d2d631f@uls.co.za>","User-Agent":"NeoMutt/20170609 (1.8.3)","X-SA-Exim-Connect-IP":"","X-SA-Exim-Mail-From":"tytso@thunk.org","X-SA-Exim-Scanned":"No (on imap.thunk.org); SAEximRunCond expanded to false","Sender":"linux-ext4-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ext4@vger.kernel.org"}},{"id":1766202,"web_url":"http://patchwork.ozlabs.org/comment/1766202/","msgid":"","list_archive_url":null,"date":"2017-09-11T10:19:05","subject":"Re: [PATCH] e2fsck: add optimization for heavily hard-linked file\n\tsystems","submitter":{"id":72203,"url":"http://patchwork.ozlabs.org/api/people/72203/","name":"Jaco Kroon","email":"jaco@uls.co.za"},"content":"Hi Theodore,\n\nI got severely side-tracked after the previous look at this.\n\nWith the mentions below, ACK.\n\nKind Regards,\nJaco\n\n\nOn 26/08/2017 17:33, Theodore Ts'o wrote:\n> On Thu, Aug 24, 2017 at 11:08:52AM +0200, Jaco Kroon wrote:\n>>> +#define E2F_OPT_ICOUNT_FULLMAP\t0x20000 /* don't optimize extents */\n>> description here seems wrong? Perhaps \"use fullmap for inode link count\" ?\n> Oops, fixed.\n>\n>>> --- a/e2fsck/pass1.c\n>>> +++ b/e2fsck/pass1.c\n>>> @@ -711,6 +711,7 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name,\n>>> \terrcode_t\t\tretval;\n>>> \tchar\t\t\t*tdb_dir;\n>>> \tint\t\t\tenable;\n>>> +\tint\t\t\tfull_map;\n>>> \t*ret = 0;\n>>> @@ -734,6 +735,8 @@ extern errcode_t e2fsck_setup_icount(e2fsck_t ctx, const char *icount_name,\n>>> \t}\n>>> \te2fsck_set_bitmap_type(ctx->fs, EXT2FS_BMAP64_RBTREE, icount_name,\n>>> \t\t\t &save_type);\n>>> +\tif (ctx->options & E2F_OPT_ICOUNT_FULLMAP)\n>>> +\t\tflags |= EXT2_ICOUNT_OPT_FULLMAP;\n>>> \tretval = ext2fs_create_icount2(ctx->fs, flags, 0, hint, ret);\n>>> \tctx->fs->default_bitmap_type = save_type;\n>>> \treturn retval;\n>> This isn't used for pass1 (I did originally use full_map either way then\n>> decided to only use it for pass2) - and we to receive flags coming into this\n>> function - shouldn't this perhaps rather go into pass2.c (e2fsck_pass2\n>> function to be more specific)? This really is cosmetic imho.\n> The e2fsck_setup_icount() function is used for both pass1 and pass2.\n>\n>>> diff --git a/e2fsck/unix.c b/e2fsck/unix.c\n>>> index ff961483b..939862f1a 100644\n>>> --- a/e2fsck/unix.c\n>>> +++ b/e2fsck/unix.c\n>>> @@ -709,9 +709,18 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)\n>>> \t\t} else if (strcmp(token, \"nodiscard\") == 0) {\n>>> \t\t\tctx->options &= ~E2F_OPT_DISCARD;\n>>> \t\t\tcontinue;\n>>> +\t\t} else if (strcmp(token, \"optimize_extents\") == 0) {\n>>> +\t\t\tctx->options &= ~E2F_OPT_NOOPT_EXTENTS;\n>>> +\t\t\tcontinue;\n>> unrelated, but makes sense :).\n> Yes, this was a cleanup I was doing as I weant.\n>\n>>> +\t\tif (!retval) {\n>>> +\t\t\tmemset(icount->fullmap, 0,\n>>> +\t\t\t*sizeof(__u32)* * fs->super->s_inodes_count); <-------\n>> this of course should be sizeof(__u16), or sizeof(*count->fullmap) which\n>> will track the type pointed to. It might be better practice to store\n>> \"sizeof(*icount->fullmap) * fs->super->s_inodes_count\" to a variable and\n>> just re-use it.\n> Good point. Fixed.\n>\n>>> \t\tgoto errout;\n>>> +\n>> Extra line can be removed.\n>>> \tif (flags & EXT2_ICOUNT_OPT_INCREMENT) {\n> Removed.\n>\n> Thanks for taking a look at the patch!\n>\n> \t\t\t\t\t\t- Ted","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-ext4-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xrP6n0CQSz9s7f\n\tfor ;\n\tMon, 11 Sep 2017 20:19:29 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751036AbdIKKT1 (ORCPT );\n\tMon, 11 Sep 2017 06:19:27 -0400","from othala.uls.co.za ([154.73.34.12]:60130 \"EHLO othala.uls.co.za\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750984AbdIKKT0 (ORCPT );\n\tMon, 11 Sep 2017 06:19:26 -0400","from uls-154-73-35-201.wall.uls.co.za ([154.73.35.201]\n\thelo=tauri.local.uls.co.za) by othala.uls.co.za with esmtpsa\n\t(TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.88)\n\t(envelope-from )\n\tid 1drLo1-00034V-Vk; Mon, 11 Sep 2017 12:19:06 +0200","from greyscale.dhcp.uls.co.za ([192.168.42.113])\n\tby tauri.local.uls.co.za with esmtpa (Exim 4.89)\n\t(envelope-from )\n\tid 1drLo1-0007Tc-BI; Mon, 11 Sep 2017 12:19:05 +0200"],"Subject":"Re: [PATCH] e2fsck: add optimization for heavily hard-linked file\n\tsystems","To":"Theodore Ts'o ","Cc":"Doug Porter , linux-ext4@vger.kernel.org,\n\tOmar Sandoval ","References":"<20170819011635.1815929-1-dsp@fb.com>\n\t<20170822022948.nyn6fessudjaj5xh@thunk.org>\n\t\n\t<20170822124505.xr7wnxonsbml3mgh@thunk.org>\n\t<00f408c1-215e-39e2-dec4-8f05eb604f97@uls.co.za>\n\t<20170823232351.643l4wg7pvnnxyts@thunk.org>\n\t<48660f5d-2e53-f0ff-e0e5-169a1d2d631f@uls.co.za>\n\t<20170826153313.le7biqxzg7mz6f32@thunk.org>","From":"Jaco Kroon ","Organization":"Ultimate Linux Solutions (Pty) Ltd","Message-ID":"","Date":"Mon, 11 Sep 2017 12:19:05 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.0","MIME-Version":"1.0","In-Reply-To":"<20170826153313.le7biqxzg7mz6f32@thunk.org>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","X-Spam-report":"Relay access (othala.uls.co.za).","Sender":"linux-ext4-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-ext4@vger.kernel.org"}}]