On Sun, 26 May 2013 16:18:20 +0400, Vyacheslav Dubeyko wrote: > Hi Ryusuke, > > On May 25, 2013, at 7:33 AM, Ryusuke Konishi wrote: > >> On Fri, 24 May 2013 17:37:12 -0400, Jörn Engel wrote: >>> On Sat, 25 May 2013 02:01:04 +0400, Vyacheslav Dubeyko wrote: >>>> On May 24, 2013, at 11:01 PM, Jörn Engel wrote: >>>>> On Fri, 24 May 2013 23:30:10 +0400, Vyacheslav Dubeyko wrote: >>>>>> On May 24, 2013, at 9:54 PM, Jörn Engel wrote: >>>>>>> On Fri, 24 May 2013 17:32:52 +0400, Vyacheslav Dubeyko wrote: >>>>>>>> >>>>>>>> Subject: [PATCH v5 2/2] nilfs2: use atomic_long_t type for inodes_count >>>>>>> ... >>>>>>>> The cp_inodes_count and cp_blocks_count are represented as >>>>>>>> __le64 type in on-disk structure (struct nilfs_checkpoint). >>>>>>> >>>>>>> Isn't atomic_long_t defined to be 32bit on 32bit architectures? >>>>>> >>>>>> As I understand, yes. >>>>> >>>>> And it doesn't concern you to use a 32bit memory structure to >>>>> represent a 64bit on-disk structure? ;) >>>> >>>> I suppose that you mean possibility to mount NILFS2 volume under 32 bit architecture >>>> after working with this volume under 64 bit architecture. Am I correct? >>>> >>>> Have you any concrete remarks about code of the patch? Feel free to offer your vision. >>> >>> Replace atomic_long_t with atomic64_t and replace the various accessor >>> functions. The on-disk data structure is 64bit, so of course you use >>> a 64bit in-memory structure. >>> >>> Does it really take a vision to come up with this idea? >> >> Things are not so simple. It is not guaranteed that 32-bit >> architectures can handle 64-bit on-disk values. (Think ino_t, page >> index, or nrpages. These are defined with "unsigned long".) >> >> However, these "inodes_count" and "blocks_count" are directly >> corresponding to their on-disk counterparts, so using atomic64_t looks >> more intuitive for most people; using atomic_long_t for these was >> confusing. >> >> Ok, I agree. >> >> Vyacheslav, could you please consider taking in Jörn's comment ? >> > > So, I have such understanding. Please, correct me if I'm wrong. > > The atomic_long_t and atomic64_t types are 64 bits on 64-bit architecture. > The atomic_long_t type is 32 bits on 32-bit architecture. So, If we want to have > 64 bits atomic counter on 32-bit architecture then we need to use generic atomic64_t > implementation (asm-generic/atomic64.t). But generic 64-bit atomic support > depends from CONFIG_GENERIC_ATOMIC64 configuration option > (http://lxr.free-electrons.com/source/include/linux/atomic.h#L127). As a result, > NILFS2 driver needs to have dependance from CONFIG_GENERIC_ATOMIC64 > configuration option on 32-bit architecture configuration. You don't have to add denpendency to CONFIG_GENERIC_ATOMIC64. In my understanding, the generic atomic64 operations (i.e. routines in lib/atomic64.c) are only used when selected architecture doesn't have own implementation. For instance, sparc32 uses the generic implementation, but x86_64 architecture uses arch/x86/include/asm/atomic64_64.h, and x86_32 uses arch/x86/include/asm/atomic64_32.h and arch/x86/lib/atomic64_{386,cx8}_32.S for 32-bit. CONFIG_GENERIC_ATOMIC64 is selected by arch/*/Kconfig in the former case. It is not intended to be selected directly by users. With regards, Ryusuke Konishi -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
