Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1282250 > unrolled thread

Re: [PATCH v2 0/4] Add online file check feature

Started byPavel Machek <pavel@ucw.cz>
First post2015-12-02 19:30 +0100
Last post2015-12-07 04:40 +0100
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 0/4] Add online file check feature Pavel Machek <pavel@ucw.cz> - 2015-12-02 19:30 +0100
    Re: [PATCH v2 0/4] Add online file check feature "Gang He" <ghe@suse.com> - 2015-12-03 03:10 +0100
      Re: [PATCH v2 0/4] Add online file check feature Greg KH <greg@kroah.com> - 2015-12-03 06:20 +0100
        Re: [PATCH v2 0/4] Add online file check feature "Gang He" <ghe@suse.com> - 2015-12-04 09:40 +0100
          Re: [PATCH v2 0/4] Add online file check feature Pavel Machek <pavel@ucw.cz> - 2015-12-04 10:30 +0100
          Re: [PATCH v2 0/4] Add online file check feature Greg KH <greg@kroah.com> - 2015-12-04 17:50 +0100
            Re: [PATCH v2 0/4] Add online file check feature "Gang He" <ghe@suse.com> - 2015-12-07 04:40 +0100

#1282250 — Re: [PATCH v2 0/4] Add online file check feature

FromPavel Machek <pavel@ucw.cz>
Date2015-12-02 19:30 +0100
SubjectRe: [PATCH v2 0/4] Add online file check feature
Message-ID<qBkka-2qa-7@gated-at.bofh.it>
On Wed 2015-10-28 14:25:57, Gang He wrote:
> When there are errors in the ocfs2 filesystem,
> they are usually accompanied by the inode number which caused the error.
> This inode number would be the input to fixing the file.
> One of these options could be considered:
> A file in the sys filesytem which would accept inode numbers.
> This could be used to communication back what has to be fixed or is fixed.
> You could write:
> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
> or
> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
> 

Are you sure this is reasonable interface? I mean.... sysfs is
supposed to be one value per file. And I don't think its suitable for
running commands.

...or returning back results.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1282644

From"Gang He" <ghe@suse.com>
Date2015-12-03 03:10 +0100
Message-ID<qBrvk-749-5@gated-at.bofh.it>
In reply to#1282250
Hello Pavel,



>>> 
> On Wed 2015-10-28 14:25:57, Gang He wrote:
>> When there are errors in the ocfs2 filesystem,
>> they are usually accompanied by the inode number which caused the error.
>> This inode number would be the input to fixing the file.
>> One of these options could be considered:
>> A file in the sys filesytem which would accept inode numbers.
>> This could be used to communication back what has to be fixed or is fixed.
>> You could write:
>> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
>> or
>> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
>> 
> 
> Are you sure this is reasonable interface? I mean.... sysfs is
> supposed to be one value per file. And I don't think its suitable for
> running commands.
Usually, the corrupted file (inode) should be rarely encountered for OCFS2 file system, then
lots of commands are executed via this interface with high performance is not expected by us.
Second, after online file check is added, we also plan to add a mount option "error=fix", that means
the file system can fix these errors automatically without a manual command triggering.

Thanks
Gang

> 
> ...or returning back results.
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek 
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html 
> .

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1282714

FromGreg KH <greg@kroah.com>
Date2015-12-03 06:20 +0100
Message-ID<qButb-Bo-7@gated-at.bofh.it>
In reply to#1282644
On Wed, Dec 02, 2015 at 07:05:27PM -0700, Gang He wrote:
> Hello Pavel,
> 
> 
> 
> >>> 
> > On Wed 2015-10-28 14:25:57, Gang He wrote:
> >> When there are errors in the ocfs2 filesystem,
> >> they are usually accompanied by the inode number which caused the error.
> >> This inode number would be the input to fixing the file.
> >> One of these options could be considered:
> >> A file in the sys filesytem which would accept inode numbers.
> >> This could be used to communication back what has to be fixed or is fixed.
> >> You could write:
> >> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
> >> or
> >> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
> >> 
> > 
> > Are you sure this is reasonable interface? I mean.... sysfs is
> > supposed to be one value per file. And I don't think its suitable for
> > running commands.
> Usually, the corrupted file (inode) should be rarely encountered for OCFS2 file system, then
> lots of commands are executed via this interface with high performance is not expected by us.
> Second, after online file check is added, we also plan to add a mount option "error=fix", that means
> the file system can fix these errors automatically without a manual command triggering.

It's not a "performance" issue, it's a "sysfs files only have one value"
type thing.  Have two files, "inode_fix" and "inode_check" and then just
write the inode into them, no need to have a "verb <inode>" type parser.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1283642

From"Gang He" <ghe@suse.com>
Date2015-12-04 09:40 +0100
Message-ID<qBU4j-h7-37@gated-at.bofh.it>
In reply to#1282714
Hi Greg,


>>> 
> On Wed, Dec 02, 2015 at 07:05:27PM -0700, Gang He wrote:
>> Hello Pavel,
>> 
>> 
>> 
>> >>> 
>> > On Wed 2015-10-28 14:25:57, Gang He wrote:
>> >> When there are errors in the ocfs2 filesystem,
>> >> they are usually accompanied by the inode number which caused the error.
>> >> This inode number would be the input to fixing the file.
>> >> One of these options could be considered:
>> >> A file in the sys filesytem which would accept inode numbers.
>> >> This could be used to communication back what has to be fixed or is fixed.
>> >> You could write:
>> >> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
>> >> or
>> >> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
>> >> 
>> > 
>> > Are you sure this is reasonable interface? I mean.... sysfs is
>> > supposed to be one value per file. And I don't think its suitable for
>> > running commands.
>> Usually, the corrupted file (inode) should be rarely encountered for OCFS2 
> file system, then
>> lots of commands are executed via this interface with high performance is 
> not expected by us.
>> Second, after online file check is added, we also plan to add a mount option 
> "error=fix", that means
>> the file system can fix these errors automatically without a manual command 
> triggering.
> 
> It's not a "performance" issue, it's a "sysfs files only have one value"
> type thing.  Have two files, "inode_fix" and "inode_check" and then just
> write the inode into them, no need to have a "verb <inode>" type parser.
Current, we have three functional items "check, fix and set", in the future, maybe we can add more item.
Then, for each functional item, we need to create a sys file and add related code (actual some code is duplicated),
I prefer to one sys file to handle multiple sub-commands.

Thanks
Gang

> 
> thanks,
> 
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1283671

FromPavel Machek <pavel@ucw.cz>
Date2015-12-04 10:30 +0100
Message-ID<qBUQH-Pe-11@gated-at.bofh.it>
In reply to#1283642
On Fri 2015-12-04 01:36:21, Gang He wrote:
> Hi Greg,
> 
> 
> >>> 
> > On Wed, Dec 02, 2015 at 07:05:27PM -0700, Gang He wrote:
> >> Hello Pavel,
> >> 
> >> 
> >> 
> >> >>> 
> >> > On Wed 2015-10-28 14:25:57, Gang He wrote:
> >> >> When there are errors in the ocfs2 filesystem,
> >> >> they are usually accompanied by the inode number which caused the error.
> >> >> This inode number would be the input to fixing the file.
> >> >> One of these options could be considered:
> >> >> A file in the sys filesytem which would accept inode numbers.
> >> >> This could be used to communication back what has to be fixed or is fixed.
> >> >> You could write:
> >> >> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
> >> >> or
> >> >> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
> >> >> 
> >> > 
> >> > Are you sure this is reasonable interface? I mean.... sysfs is
> >> > supposed to be one value per file. And I don't think its suitable for
> >> > running commands.
> >> Usually, the corrupted file (inode) should be rarely encountered for OCFS2 
> > file system, then
> >> lots of commands are executed via this interface with high performance is 
> > not expected by us.
> >> Second, after online file check is added, we also plan to add a mount option 
> > "error=fix", that means
> >> the file system can fix these errors automatically without a manual command 
> > triggering.
> > 
> > It's not a "performance" issue, it's a "sysfs files only have one value"
> > type thing.  Have two files, "inode_fix" and "inode_check" and then just
> > write the inode into them, no need to have a "verb <inode>" type parser.
> Current, we have three functional items "check, fix and set", in the future, maybe we can add more item.
> Then, for each functional item, we need to create a sys file and add related code (actual some code is duplicated),
> I prefer to one sys file to handle multiple sub-commands.

And we prefer not to have your code in tree.

Please design some reasonable interface. Abusing sysfs for this is not
right.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284013

FromGreg KH <greg@kroah.com>
Date2015-12-04 17:50 +0100
Message-ID<qC1Iu-5b5-33@gated-at.bofh.it>
In reply to#1283642
On Fri, Dec 04, 2015 at 01:36:21AM -0700, Gang He wrote:
> Hi Greg,
> 
> 
> >>> 
> > On Wed, Dec 02, 2015 at 07:05:27PM -0700, Gang He wrote:
> >> Hello Pavel,
> >> 
> >> 
> >> 
> >> >>> 
> >> > On Wed 2015-10-28 14:25:57, Gang He wrote:
> >> >> When there are errors in the ocfs2 filesystem,
> >> >> they are usually accompanied by the inode number which caused the error.
> >> >> This inode number would be the input to fixing the file.
> >> >> One of these options could be considered:
> >> >> A file in the sys filesytem which would accept inode numbers.
> >> >> This could be used to communication back what has to be fixed or is fixed.
> >> >> You could write:
> >> >> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
> >> >> or
> >> >> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
> >> >> 
> >> > 
> >> > Are you sure this is reasonable interface? I mean.... sysfs is
> >> > supposed to be one value per file. And I don't think its suitable for
> >> > running commands.
> >> Usually, the corrupted file (inode) should be rarely encountered for OCFS2 
> > file system, then
> >> lots of commands are executed via this interface with high performance is 
> > not expected by us.
> >> Second, after online file check is added, we also plan to add a mount option 
> > "error=fix", that means
> >> the file system can fix these errors automatically without a manual command 
> > triggering.
> > 
> > It's not a "performance" issue, it's a "sysfs files only have one value"
> > type thing.  Have two files, "inode_fix" and "inode_check" and then just
> > write the inode into them, no need to have a "verb <inode>" type parser.
> Current, we have three functional items "check, fix and set", in the future, maybe we can add more item.
> Then, for each functional item, we need to create a sys file and add related code (actual some code is duplicated),
> I prefer to one sys file to handle multiple sub-commands.

No, sorry, that is not how sysfs works.  Please use individual files,
again, sysfs is "one value per file" you should never have to write a
"parser" for a sysfs file either reading, or writing to it.

If you need additional things in the future, great, add new sysfs files,
that makes it the easiest way for your userspace tools to be able to
determine if that feature is present in the kernel or not, it does not
have to write a command that it doesn't know if the kernel can handle or
not.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1284987

From"Gang He" <ghe@suse.com>
Date2015-12-07 04:40 +0100
Message-ID<qCUOB-7x0-11@gated-at.bofh.it>
In reply to#1284013
Hello Greg and Pavel,

Sorry, there was a misunderstand, I was not aware that there were some design constraints for sysfs interfaces. I will review and modify this portion code. 


Thanks
Gang 


>>> 
> On Fri, Dec 04, 2015 at 01:36:21AM -0700, Gang He wrote:
>> Hi Greg,
>> 
>> 
>> >>> 
>> > On Wed, Dec 02, 2015 at 07:05:27PM -0700, Gang He wrote:
>> >> Hello Pavel,
>> >> 
>> >> 
>> >> 
>> >> >>> 
>> >> > On Wed 2015-10-28 14:25:57, Gang He wrote:
>> >> >> When there are errors in the ocfs2 filesystem,
>> >> >> they are usually accompanied by the inode number which caused the error.
>> >> >> This inode number would be the input to fixing the file.
>> >> >> One of these options could be considered:
>> >> >> A file in the sys filesytem which would accept inode numbers.
>> >> >> This could be used to communication back what has to be fixed or is fixed.
>> >> >> You could write:
>> >> >> $# echo "CHECK <inode>" > /sys/fs/ocfs2/devname/filecheck
>> >> >> or
>> >> >> $# echo "FIX <inode>" > /sys/fs/ocfs2/devname/filecheck
>> >> >> 
>> >> > 
>> >> > Are you sure this is reasonable interface? I mean.... sysfs is
>> >> > supposed to be one value per file. And I don't think its suitable for
>> >> > running commands.
>> >> Usually, the corrupted file (inode) should be rarely encountered for OCFS2 
>> > file system, then
>> >> lots of commands are executed via this interface with high performance is 
>> > not expected by us.
>> >> Second, after online file check is added, we also plan to add a mount 
> option 
>> > "error=fix", that means
>> >> the file system can fix these errors automatically without a manual command 
> 
>> > triggering.
>> > 
>> > It's not a "performance" issue, it's a "sysfs files only have one value"
>> > type thing.  Have two files, "inode_fix" and "inode_check" and then just
>> > write the inode into them, no need to have a "verb <inode>" type parser.
>> Current, we have three functional items "check, fix and set", in the future, 
> maybe we can add more item.
>> Then, for each functional item, we need to create a sys file and add related 
> code (actual some code is duplicated),
>> I prefer to one sys file to handle multiple sub-commands.
> 
> No, sorry, that is not how sysfs works.  Please use individual files,
> again, sysfs is "one value per file" you should never have to write a
> "parser" for a sysfs file either reading, or writing to it.
> 
> If you need additional things in the future, great, add new sysfs files,
> that makes it the easiest way for your userspace tools to be able to
> determine if that feature is present in the kernel or not, it does not
> have to write a command that it doesn't know if the kernel can handle or
> not.
> 
> thanks,
> 
> greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web