All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Vyacheslav Dubeyko <slava@dubeyko.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 01/41] fs/adfs: inode: update timestamps to centisecond precision
Date: Mon, 9 Dec 2019 14:40:00 +0000	[thread overview]
Message-ID: <20191209143959.GL25745@shell.armlinux.org.uk> (raw)
In-Reply-To: <e45222ab3f6292c013c93126078396f4b212d904.camel@dubeyko.com>

On Mon, Dec 09, 2019 at 05:34:05PM +0300, Vyacheslav Dubeyko wrote:
> On Mon, 2019-12-09 at 14:03 +0000, Russell King - ARM Linux admin
> wrote:
> > > > 
> > On Mon, Dec 09, 2019 at 04:54:55PM +0300, Vyacheslav Dubeyko wrote:
> > > On Mon, 2019-12-09 at 11:08 +0000, Russell King wrote:
> 
> <snipped>
> 
> > > >  	sb->s_fs_info = asb;
> > > > +	sb->s_time_gran = 10000000;
> > > 
> > > I believe it's not easy to follow what this granularity means.
> > > Maybe,
> > > it makes sense to introduce some constant and to add some comment?
> > 
> > Or simply name it "s_time_gran_ns" so the units are in the name.
> > 
> 
> Sounds good. :) But why namely 10000000?

I don't know what you mean.

If you're asking, why "10000000", isn't it obvious if you read the
commit message?  adfs has "centi-second" granularity. s_time_gran
is in nanoseconds. There are 10000000 nanoseconds in a centisecond.

What do you expect?

#define ADFS_TIME_GRAN 10000000

	sb->s_time_gran = ADFS_TIME_GRAN;

?

How does that help - it just stupidly and needlessly obfuscates the
code.

The whole "use definitions for constants" is idiotic when a constant
is only used in one place - when it means you have to search through
more source code to find it's single definition. Sorry, I'm not
doing that and make readability *worse*.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

  reply	other threads:[~2019-12-09 14:40 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09 11:07 [PATCH 00/41] fs/adfs updates for 5.6 Russell King - ARM Linux admin
2019-12-09 11:08 ` [PATCH 01/41] fs/adfs: inode: update timestamps to centisecond precision Russell King
2019-12-09 13:54   ` Vyacheslav Dubeyko
2019-12-09 14:03     ` Russell King - ARM Linux admin
2019-12-09 14:34       ` Vyacheslav Dubeyko
2019-12-09 14:40         ` Russell King - ARM Linux admin [this message]
2019-12-09 17:15           ` Matthew Wilcox
2019-12-09 11:08 ` [PATCH 02/41] fs/adfs: inode: fix adfs_mode2atts() Russell King
2019-12-09 11:08 ` [PATCH 03/41] fs/adfs: map: move map reading and validation to map.c Russell King
2019-12-09 11:08 ` [PATCH 04/41] fs/adfs: map: rename adfs_map_free() to adfs_map_statfs() Russell King
2019-12-09 11:08 ` [PATCH 05/41] fs/adfs: map: break up adfs_read_map() Russell King
2019-12-09 11:08 ` [PATCH 06/41] fs/adfs: map: factor out map cleanup Russell King
2019-12-09 11:08 ` [PATCH 07/41] fs/adfs: map: incorporate map offsets into layout Russell King
2019-12-09 11:08 ` [PATCH 08/41] fs/adfs: map: use find_next_bit_le() rather than open coding it Russell King
2019-12-09 11:08 ` [PATCH 09/41] fs/adfs: map: move map-specific sb initialisation to map.c Russell King
2019-12-09 11:09 ` [PATCH 10/41] fs/adfs: map: fix map scanning Russell King
2019-12-09 11:09 ` [PATCH 11/41] fs/adfs: dir: rename bh_fplus to bhs Russell King
2019-12-09 11:09 ` [PATCH 12/41] fs/adfs: dir: add common dir object initialisation Russell King
2019-12-09 11:09 ` [PATCH 13/41] fs/adfs: dir: add common directory buffer release method Russell King
2019-12-09 11:09 ` [PATCH 14/41] fs/adfs: dir: add common directory sync method Russell King
2019-12-09 11:09 ` [PATCH 15/41] fs/adfs: dir: add generic copy functions Russell King
2019-12-09 11:09 ` [PATCH 16/41] fs/adfs: dir: add generic directory reading Russell King
2019-12-09 11:09 ` [PATCH 17/41] fs/adfs: dir: add helper to read directory using inode Russell King
2019-12-09 11:09 ` [PATCH 18/41] fs/adfs: dir: add helper to mark directory buffers dirty Russell King
2019-12-09 11:09 ` [PATCH 19/41] fs/adfs: dir: update directory locking Russell King
2019-12-09 11:09 ` [PATCH 20/41] fs/adfs: dir: modernise on-disk directory structures Russell King
2019-12-09 11:10 ` [PATCH 21/41] fs/adfs: dir: improve update failure handling Russell King
2019-12-09 11:10 ` [PATCH 22/41] fs/adfs: dir: improve compiler coverage in adfs_dir_update Russell King
2019-12-09 11:10 ` [PATCH 23/41] fs/adfs: dir: switch to iterate_shared method Russell King
2019-12-09 11:10 ` [PATCH 24/41] fs/adfs: dir: add more efficient iterate() per-format method Russell King
2019-12-09 11:10 ` [PATCH 25/41] fs/adfs: dir: use pointers to access directory head/tails Russell King
2019-12-09 11:10 ` [PATCH 26/41] fs/adfs: newdir: factor out directory format validation Russell King
2019-12-09 11:10 ` [PATCH 27/41] fs/adfs: newdir: improve directory validation Russell King
2019-12-09 11:10 ` [PATCH 28/41] fs/adfs: newdir: merge adfs_dir_read() into adfs_f_read() Russell King
2019-12-09 11:10 ` [PATCH 29/41] fs/adfs: newdir: clean up adfs_f_update() Russell King
2019-12-09 11:10 ` [PATCH 30/41] fs/adfs: newdir: split out directory commit from update Russell King
2019-12-09 11:10 ` [PATCH 31/41] fs/adfs: bigdir: factor out directory entry offset calculation Russell King
2019-12-09 11:10 ` [PATCH 32/41] fs/adfs: bigdir: extract directory validation Russell King
2019-12-09 11:11 ` [PATCH 33/41] fs/adfs: bigdir: directory validation strengthening Russell King
2019-12-09 11:11 ` [PATCH 34/41] fs/adfs: bigdir: calculate and validate directory checkbyte Russell King
2019-12-09 11:11 ` [PATCH 35/41] fs/adfs: bigdir: implement directory update support Russell King
2019-12-09 11:11 ` [PATCH 36/41] fs/adfs: super: fix inode dropping Russell King
2019-12-09 11:11 ` [PATCH 37/41] fs/adfs: dir: remove debug in adfs_dir_update() Russell King
2019-12-09 11:11 ` [PATCH 38/41] fs/adfs: super: extract filesystem block probe Russell King
2019-12-09 11:11 ` [PATCH 39/41] fs/adfs: super: add support for E and E+ floppy image formats Russell King
2019-12-09 11:11 ` [PATCH 40/41] fs/adfs: mostly divorse inode number from indirect disc address Russell King
2019-12-09 11:11 ` [PATCH 41/41] Documentation: update adfs filesystem documentation Russell King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191209143959.GL25745@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=slava@dubeyko.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.