* [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name
@ 2013-11-27 9:05 Joseph Qi
2013-11-27 23:38 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Joseph Qi @ 2013-11-27 9:05 UTC (permalink / raw)
To: ocfs2-devel
Dentry lock name is made up of the type 'N', the parent blkno and its
blkno represented as a binary value.
Now there is a null byte between parent blkno and the binary blkno
value, so drop it.
Signed-off-by: Joseph Qi <joseph.qi@huawei.com>
---
fs/ocfs2/dlm/dlmdebug.c | 2 +-
fs/ocfs2/dlmglue.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
index e33cd7a..2d85dfa 100644
--- a/fs/ocfs2/dlm/dlmdebug.c
+++ b/fs/ocfs2/dlm/dlmdebug.c
@@ -257,7 +257,7 @@ static int stringify_lockname(const char *lockname, int locklen, char *buf,
#define OCFS2_DENTRY_LOCK_INO_START 18
if (*lockname == 'N') {
memcpy((__be64 *)&inode_blkno_be,
- (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START],
+ (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START - 1],
sizeof(__be64));
out += snprintf(buf + out, len - out, "%.*s%08x",
OCFS2_DENTRY_LOCK_INO_START - 1, lockname,
diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
index 3a44a64..62f2387 100644
--- a/fs/ocfs2/dlmglue.c
+++ b/fs/ocfs2/dlmglue.c
@@ -586,7 +586,7 @@ static __u64 ocfs2_get_dentry_lock_ino(struct ocfs2_lock_res *lockres)
{
__be64 inode_blkno_be;
- memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START],
+ memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1],
sizeof(__be64));
return be64_to_cpu(inode_blkno_be);
@@ -631,7 +631,7 @@ void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
BUG_ON(len != (OCFS2_DENTRY_LOCK_INO_START - 1));
- memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START], &inode_blkno_be,
+ memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1], &inode_blkno_be,
sizeof(__be64));
ocfs2_lock_res_init_common(OCFS2_SB(inode->i_sb), lockres,
--
1.8.4.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name
2013-11-27 9:05 [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name Joseph Qi
@ 2013-11-27 23:38 ` Andrew Morton
2013-11-28 1:03 ` Joseph Qi
2013-11-28 16:40 ` Mark Fasheh
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2013-11-27 23:38 UTC (permalink / raw)
To: ocfs2-devel
On Wed, 27 Nov 2013 17:05:00 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
> Dentry lock name is made up of the type 'N', the parent blkno and its
> blkno represented as a binary value.
> Now there is a null byte between parent blkno and the binary blkno
> value, so drop it.
Geeze.
What are the runtime effects of this change?
> --- a/fs/ocfs2/dlm/dlmdebug.c
> +++ b/fs/ocfs2/dlm/dlmdebug.c
> @@ -257,7 +257,7 @@ static int stringify_lockname(const char *lockname, int locklen, char *buf,
> #define OCFS2_DENTRY_LOCK_INO_START 18
> if (*lockname == 'N') {
> memcpy((__be64 *)&inode_blkno_be,
> - (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START],
> + (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START - 1],
And what's up with all the CrazyCasting? &inode_blkno_be is *already*
__be64* and the second arg to memcpy is `const void *', so the cast
there is also unneeded.
> sizeof(__be64));
> out += snprintf(buf + out, len - out, "%.*s%08x",
> OCFS2_DENTRY_LOCK_INO_START - 1, lockname,
> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
> index 3a44a64..62f2387 100644
> --- a/fs/ocfs2/dlmglue.c
> +++ b/fs/ocfs2/dlmglue.c
> @@ -586,7 +586,7 @@ static __u64 ocfs2_get_dentry_lock_ino(struct ocfs2_lock_res *lockres)
> {
> __be64 inode_blkno_be;
>
> - memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START],
> + memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1],
Like that.
> sizeof(__be64));
>
> return be64_to_cpu(inode_blkno_be);
> @@ -631,7 +631,7 @@ void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
>
> BUG_ON(len != (OCFS2_DENTRY_LOCK_INO_START - 1));
>
> - memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START], &inode_blkno_be,
> + memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1], &inode_blkno_be,
> sizeof(__be64));
>
> ocfs2_lock_res_init_common(OCFS2_SB(inode->i_sb), lockres,
> --
> 1.8.4.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name
2013-11-27 23:38 ` Andrew Morton
@ 2013-11-28 1:03 ` Joseph Qi
2013-11-28 16:40 ` Mark Fasheh
1 sibling, 0 replies; 6+ messages in thread
From: Joseph Qi @ 2013-11-28 1:03 UTC (permalink / raw)
To: ocfs2-devel
On 2013/11/28 7:38, Andrew Morton wrote:
> On Wed, 27 Nov 2013 17:05:00 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
>
>> Dentry lock name is made up of the type 'N', the parent blkno and its
>> blkno represented as a binary value.
>> Now there is a null byte between parent blkno and the binary blkno
>> value, so drop it.
>
> Geeze.
>
> What are the runtime effects of this change?
>
If the null byte exists, it is obviously we cannot print the full dentry
lock name using '%s'.
But the fact is, the blkno is stuffed with binary value, it may also
contain null bytes, so we still cannot get the full dentry lock name in
the message.
Maybe we should also distinguish dentry lock and other lockres when
print lock name. But this makes the code malformed.
>> --- a/fs/ocfs2/dlm/dlmdebug.c
>> +++ b/fs/ocfs2/dlm/dlmdebug.c
>> @@ -257,7 +257,7 @@ static int stringify_lockname(const char *lockname, int locklen, char *buf,
>> #define OCFS2_DENTRY_LOCK_INO_START 18
>> if (*lockname == 'N') {
>> memcpy((__be64 *)&inode_blkno_be,
>> - (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START],
>> + (char *)&lockname[OCFS2_DENTRY_LOCK_INO_START - 1],
>
> And what's up with all the CrazyCasting? &inode_blkno_be is *already*
> __be64* and the second arg to memcpy is `const void *', so the cast
> there is also unneeded.
>
>> sizeof(__be64));
>> out += snprintf(buf + out, len - out, "%.*s%08x",
>> OCFS2_DENTRY_LOCK_INO_START - 1, lockname,
>> diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c
>> index 3a44a64..62f2387 100644
>> --- a/fs/ocfs2/dlmglue.c
>> +++ b/fs/ocfs2/dlmglue.c
>> @@ -586,7 +586,7 @@ static __u64 ocfs2_get_dentry_lock_ino(struct ocfs2_lock_res *lockres)
>> {
>> __be64 inode_blkno_be;
>>
>> - memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START],
>> + memcpy(&inode_blkno_be, &lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1],
>
> Like that.
>
>> sizeof(__be64));
>>
>> return be64_to_cpu(inode_blkno_be);
>> @@ -631,7 +631,7 @@ void ocfs2_dentry_lock_res_init(struct ocfs2_dentry_lock *dl,
>>
>> BUG_ON(len != (OCFS2_DENTRY_LOCK_INO_START - 1));
>>
>> - memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START], &inode_blkno_be,
>> + memcpy(&lockres->l_name[OCFS2_DENTRY_LOCK_INO_START - 1], &inode_blkno_be,
>> sizeof(__be64));
>>
>> ocfs2_lock_res_init_common(OCFS2_SB(inode->i_sb), lockres,
>> --
>> 1.8.4.3
>
> .
>
^ permalink raw reply [flat|nested] 6+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name
2013-11-27 23:38 ` Andrew Morton
2013-11-28 1:03 ` Joseph Qi
@ 2013-11-28 16:40 ` Mark Fasheh
2013-11-30 1:45 ` Joseph Qi
1 sibling, 1 reply; 6+ messages in thread
From: Mark Fasheh @ 2013-11-28 16:40 UTC (permalink / raw)
To: ocfs2-devel
On Wed, Nov 27, 2013 at 03:38:41PM -0800, Andrew Morton wrote:
> On Wed, 27 Nov 2013 17:05:00 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
>
> > Dentry lock name is made up of the type 'N', the parent blkno and its
> > blkno represented as a binary value.
> > Now there is a null byte between parent blkno and the binary blkno
> > value, so drop it.
>
> Geeze.
>
> What are the runtime effects of this change?
This will change the over-the-wire protocol, so big NACK for this
patch please. Some of the lock names are binary and have been forever now.
We have a limited amount of space in the name and some lock types would have
names too long if we stringified every field.
--Mark
--
Mark Fasheh
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name
2013-11-28 16:40 ` Mark Fasheh
@ 2013-11-30 1:45 ` Joseph Qi
2013-12-01 23:16 ` Mark Fasheh
0 siblings, 1 reply; 6+ messages in thread
From: Joseph Qi @ 2013-11-30 1:45 UTC (permalink / raw)
To: ocfs2-devel
On 2013/11/29 0:40, Mark Fasheh wrote:
> On Wed, Nov 27, 2013 at 03:38:41PM -0800, Andrew Morton wrote:
>> On Wed, 27 Nov 2013 17:05:00 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
>>
>>> Dentry lock name is made up of the type 'N', the parent blkno and its
>>> blkno represented as a binary value.
>>> Now there is a null byte between parent blkno and the binary blkno
>>> value, so drop it.
>>
>> Geeze.
>>
>> What are the runtime effects of this change?
>
> This will change the over-the-wire protocol, so big NACK for this
> patch please. Some of the lock names are binary and have been forever now.
> We have a limited amount of space in the name and some lock types would have
> names too long if we stringified every field.
> --Mark
>
What do you mean by change the over-the-wire protocol?
In my opinion, the only thing we should do is to make buildup and parse
consistent.
But if the new version is mixed mounted with the old, it may have
problem because of the changed protocol.
> --
> Mark Fasheh
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name
2013-11-30 1:45 ` Joseph Qi
@ 2013-12-01 23:16 ` Mark Fasheh
0 siblings, 0 replies; 6+ messages in thread
From: Mark Fasheh @ 2013-12-01 23:16 UTC (permalink / raw)
To: ocfs2-devel
On Sat, Nov 30, 2013 at 09:45:04AM +0800, Joseph Qi wrote:
> On 2013/11/29 0:40, Mark Fasheh wrote:
> > On Wed, Nov 27, 2013 at 03:38:41PM -0800, Andrew Morton wrote:
> >> On Wed, 27 Nov 2013 17:05:00 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
> >>
> >>> Dentry lock name is made up of the type 'N', the parent blkno and its
> >>> blkno represented as a binary value.
> >>> Now there is a null byte between parent blkno and the binary blkno
> >>> value, so drop it.
> >>
> >> Geeze.
> >>
> >> What are the runtime effects of this change?
> >
> > This will change the over-the-wire protocol, so big NACK for this
> > patch please. Some of the lock names are binary and have been forever now.
> > We have a limited amount of space in the name and some lock types would have
> > names too long if we stringified every field.
> > --Mark
> >
> What do you mean by change the over-the-wire protocol?
> In my opinion, the only thing we should do is to make buildup and parse
> consistent.
> But if the new version is mixed mounted with the old, it may have
> problem because of the changed protocol.
That's exactly what "change the over-the-wire protocol" means - "new" and
old versions of the file will not be able to communicate. This is
unacceptable and will anger our users.
We don't make changes like that just to clean up fields.
--Mark
--
Mark Fasheh
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-01 23:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 9:05 [Ocfs2-devel] [PATCH] ocfs2: drop the unwanted null byte in dentry lock name Joseph Qi
2013-11-27 23:38 ` Andrew Morton
2013-11-28 1:03 ` Joseph Qi
2013-11-28 16:40 ` Mark Fasheh
2013-11-30 1:45 ` Joseph Qi
2013-12-01 23:16 ` Mark Fasheh
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.