LU-3677 mdt: Set HSM dirty open-for-write file when evicted.
PATCH 1/1 LU-3677 mdt: Set HSM dirty open-for-write file when evicted.
Fix regression introduced by LU-1303 (5165cdd). Previously, MDS_CLOSE_CLEANUP was used to detect file closing, due to eviction. This flag is no more since 5165cdd.
It completely removed this symbol.
Reviewed-on: http://review.whamcloud.com/7195
--- .../lustre/lustre/include/lustre/lustre_idl.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/staging/lustre/lustre/include/lustre/lustre_idl.h +1 -1
@@ -2390,7 +2390,7 @@ enum mds_op_bias {MDS_PERM_BYPASS =1 <<3 ,MDS_SOM =1 <<4 ,MDS_QUOTA_IGNORE =1 <<5 , -MDS_CLOSE_CLEANUP =1 <<6 , +/* Was MDS_CLOSE_CLEANUP = (1 << 6), No more used */ MDS_KEEP_ORPHAN =1 <<7 ,MDS_RECOV_OPEN =1 <<8 , MDS_DATA_MODIFIED = 1 << 9,
Signature · 1 line
1.6.5.6
Dilger
PATCH 1/1 LU-3677 mdt: Set HSM dirty open-for-write file when evicted.
Hi Ben,
for patches being submitted to the upstream kernel, they also need to be CC'd to the maintainers and lists of the "Linux Staging" project. Since we're pretty new at taking outside patches for upstream, I'll let Oleg comment on whether he prefers to accumulate the patches himself and pass them upstream, or if you should CC the upstream maintainers/lists directly.
That said, since you are doing this for the first time it makes sense to just send the patch to lustre-devel and we can iterate until the patch is ready to go upstream. A few comments inline.
Quoted 1 line · Ben Evans
LU-3677 mdt: Set HSM dirty open-for-write file when evicted.
For patches being pushed upstream, the "LU-3677" label should be removed from the subject line to a separate tag at the end after Reviewed-on:
Intel-bug-ID: http://jira.hpdd.intel.com/browse/LU-3677
Quoted 3 lines
Fix regression introduced by LU-1303 (5165cdd). Previously,
MDS_CLOSE_CLEANUPwas used to detect file closing, due to eviction. This flag is no more since 5165cdd.
The patch commit hash referenced here (5165cdd) is only relevant for the Lustre "master" branch, but do not make any sense for the kernel tree itself (where this patch is going), and the upstream maintainers don't like that. Since the referenced patch was landed before the Lustre client code was pushed upstream, it probably doesn't make sense to reference this at all, and just update the commit comment like:
Previously, MDS_CLOSE_CLEANUP was used to detect file closing due to eviction. This flag is not used anymore.
Completely remove this symbol.
Quoted 1 line
Change-Id: I20e18103fe085672f499c956e831564e45bd5200
Upstream doesn't like the Change-Id, since it is confusing for other Gerrit instances if the patches are being imported. This should be flagged if checkpatch.pl is run against the patch, which should be done for every patch going upstream, even if it was landed to master previously.
Quoted 1 line
Reviewed-on: http://review.whamcloud.com/7195
Reviewed-on: is OK.
Quoted 2 lines
Tested-by HudsonTested-by Maloo <[email protected]>
Remove Tested-by: since those don't mean anything to upstream.
Quoted 6 lines
Signed-off-by Aurelien Degremont <[email protected]>Reviewed-by Andreas Dilger <[email protected]>Reviewed-by Jinshan Xiong <[email protected]>Reviewed-by John L. Hammond <[email protected]>Reviewed-by Fan Yong <[email protected]>Reviewed-by Oleg Drokin <[email protected]>
The Signed-off-by: and Reviewed-by: lines should stay.
As you can see, the commit message and possibly the patch itself may be slightly different between master and upstream, but at least with the Reviewed-on: and Intel-bug-id: tags we can track the patches between the two.
There are similar labels needed when porting patches from upstream kernels into master.
Both of these cases are already described on the wiki page:
https://wiki.hpdd.intel.com/display/PUB/Commit+Comments under the "Additional commit tags" section.
Cheers, Andreas
Quoted 26 lines
---
.../lustre/lustre/include/lustre/lustre_idl.h| 2 +-1 files changed, 1 insertions(+), 1 deletions(-)drivers/staging/lustre/lustre/include/lustre/lustre_idl.h +1
-1@@ -2390,7 +2390,7 @@ enum mds_op_bias {MDS_PERM_BYPASS =1 <<3 ,MDS_SOM =1 <<4 ,MDS_QUOTA_IGNORE =1 <<5 , -MDS_CLOSE_CLEANUP =1 <<6 , +/* Was MDS_CLOSE_CLEANUP = (1 << 6), No more used */ MDS_KEEP_ORPHAN =1 <<7 ,MDS_RECOV_OPEN =1 <<8 , MDS_DATA_MODIFIED = 1 << 9,Signature · 6 lines
1.6.5.6 _______________________________________________ lustre-devel mailing list lustre-devel at lists.lustre.org http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
Cheers, Andreas
Signature · 4 lines
Andreas Dilger Lustre Software Architect Intel High Performance Data Division
Drokin
PATCH 1/1 LU-3677 mdt: Set HSM dirty open-for-write file when evicted.
Hello!
Quoted 6 lines · Andreas
Hi Ben,
for patches being submitted to the upstream kernel, they also need to be CC'd to the maintainers and lists of the "Linux Staging" project. Since we're pretty new at taking outside patches for upstream, I'll let Oleg comment on whether he prefers to accumulate the patches himself and pass them upstream, or if you should CC the upstream maintainers/lists directly.
I think it's ok for people to send patches themselves.
Quoted 8 lines · Ben Evans
Quoted 2 lines
LU-3677 mdt: Set HSM dirty open-for-write file when evicted.
For patches being pushed upstream, the "LU-3677" label should be removed from the subject line to a separate tag at the end after Reviewed-on:
Intel-bug-ID: http://jira.hpdd.intel.com/browse/LU-3677
Additionally instead of the subsystem (mdt in this instance), I typically expand that some more to something like:
staging/lustre/mdt (or jsut staging/lustre if there's no clear subsystem).
also note taht mdt is not really a subsystem in upstream kernel so typically patches to it are skipped from being sent upstream or redone to only retain relevant client-side bits.
Quoted 33 lines
Quoted 3 lines
Fix regression introduced by LU-1303 (5165cdd). Previously,
MDS_CLOSE_CLEANUPwas used to detect file closing, due to eviction. This flag is no more since 5165cdd.The patch commit hash referenced here (5165cdd) is only relevant for the Lustre "master" branch, but do not make any sense for the kernel tree itself (where this patch is going), and the upstream maintainers don't like that. Since the referenced patch was landed before the Lustre client code was pushed upstream, it probably doesn't make sense to reference this at all, and just update the commit comment like:
Previously,
MDS_CLOSE_CLEANUPwas used to detect file closing due to eviction. This flag is not used anymore.Completely remove this symbol.
Quoted 1 line
Change-Id: I20e18103fe085672f499c956e831564e45bd5200
Upstream doesn't like the Change-Id, since it is confusing for other Gerrit instances if the patches are being imported. This should be flagged if checkpatch.pl is run against the patch, which should be done for every patch going upstream, even if it was landed to master previously.
Quoted 1 line
Reviewed-on: http://review.whamcloud.com/7195
Reviewed-on: is OK.
Quoted 2 lines
Tested-by HudsonTested-by Maloo <[email protected]>Remove Tested-by: since those don't mean anything to upstream.
Quoted 8 lines
Quoted 6 lines
Signed-off-by Aurelien Degremont <[email protected]>Reviewed-by Andreas Dilger <[email protected]>Reviewed-by Jinshan Xiong <[email protected]>Reviewed-by John L. Hammond <[email protected]>Reviewed-by Fan Yong <[email protected]>Reviewed-by Oleg Drokin <[email protected]>The Signed-off-by: and Reviewed-by: lines should stay.
Also your own Signed-off-by: line needs to appear here at the end.
Quoted 18 lines
As you can see, the commit message and possibly the patch itself may be slightly different between master and upstream, but at least with the Reviewed-on: and Intel-bug-id: tags we can track the patches between the two.
There are similar labels needed when porting patches from upstream kernels into master.
Both of these cases are already described on the wiki page:
https://wiki.hpdd.intel.com/display/PUB/Commit+Comments under the "Additional commit tags" section.Cheers, Andreas
Quoted 4 lines
---
.../lustre/lustre/include/lustre/lustre_idl.h| 2 +-1 files changed, 1 insertions(+), 1 deletions(-)
Quoted 11 lines
drivers/staging/lustre/lustre/include/lustre/lustre_idl.h enum mds_op_bias { · line 2393
Quoted 11 lines
drivers/staging/lustre/lustre/include/lustre/lustre_idl.h +1
-1@@ -2390,7 +2390,7 @@ enum mds_op_bias {MDS_PERM_BYPASS =1 <<3 ,MDS_SOM =1 <<4 , MDS_QUOTA_IGNORE = 1 << 5, - MDS_CLOSE_CLEANUP = 1 << 6, +/* Was MDS_CLOSE_CLEANUP = (1 << 6), No more used */
I imagine you can also fix spelling/grammar as you go (submitting symmetric patches to master).
This would likely be fixed in upstream by others creating further drift, so probably a good idea to take it under control from the get go.
Thanks.
Bye,
Oleg