On 6/13/23 17:07, Ilya Dryomov wrote:
On Mon, Jun 12, 2023 at 1:46 PM <xiubli@xxxxxxxxxx> wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>
Multiple cephfs mounts on a host is increasingly common so disambiguating
messages like this is necessary and will make it easier to debug
issues.
URL: https://tracker.ceph.com/issues/61590
Cc: Patrick Donnelly <pdonnell@xxxxxxxxxx>
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
fs/ceph/acl.c | 6 +-
fs/ceph/addr.c | 300 ++++++++++--------
fs/ceph/caps.c | 709 ++++++++++++++++++++++++-------------------
fs/ceph/crypto.c | 45 ++-
fs/ceph/debugfs.c | 4 +-
fs/ceph/dir.c | 222 +++++++++-----
fs/ceph/export.c | 39 ++-
fs/ceph/file.c | 268 +++++++++-------
fs/ceph/inode.c | 528 ++++++++++++++++++--------------
fs/ceph/ioctl.c | 10 +-
fs/ceph/locks.c | 62 ++--
fs/ceph/mds_client.c | 616 +++++++++++++++++++++----------------
fs/ceph/mdsmap.c | 25 +-
fs/ceph/metric.c | 5 +-
fs/ceph/quota.c | 31 +-
fs/ceph/snap.c | 186 +++++++-----
fs/ceph/super.c | 64 ++--
fs/ceph/xattr.c | 97 +++---
18 files changed, 1887 insertions(+), 1330 deletions(-)
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index 8a56f979c7cb..970acd07908d 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include "super.h"
+#include "mds_client.h"
static inline void ceph_set_cached_acl(struct inode *inode,
int type, struct posix_acl *acl)
@@ -31,6 +32,7 @@ static inline void ceph_set_cached_acl(struct inode *inode,
struct posix_acl *ceph_get_acl(struct inode *inode, int type, bool rcu)
{
+ struct ceph_client *cl = ceph_inode_to_client(inode);
int size;
unsigned int retry_cnt = 0;
const char *name;
@@ -72,8 +74,8 @@ struct posix_acl *ceph_get_acl(struct inode *inode, int type, bool rcu)
} else if (size == -ENODATA || size == 0) {
acl = NULL;
} else {
- pr_err_ratelimited("get acl %llx.%llx failed, err=%d\n",
- ceph_vinop(inode), size);
+ pr_err_ratelimited_client(cl, "%s %llx.%llx failed, err=%d\n",
+ __func__, ceph_vinop(inode), size);
acl = ERR_PTR(-EIO);
}
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index e62318b3e13d..c772639dc0cb 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -79,18 +79,18 @@ static inline struct ceph_snap_context *page_snap_context(struct page *page)
*/
static bool ceph_dirty_folio(struct address_space *mapping, struct folio *folio)
{
- struct inode *inode;
+ struct inode *inode = mapping->host;
+ struct ceph_client *cl = ceph_inode_to_client(inode);
struct ceph_inode_info *ci;
struct ceph_snap_context *snapc;
if (folio_test_dirty(folio)) {
- dout("%p dirty_folio %p idx %lu -- already dirty\n",
- mapping->host, folio, folio->index);
+ dout_client(cl, "%s %llx.%llx %p idx %lu -- already dirty\n",
+ __func__, ceph_vinop(inode), folio, folio->index);
While having context information attached to each dout is nice, it
certainly comes at a price of a lot of churn and automated backport
disruption.
Yeah, certainly this will break automated backporting. But this should
be okay, I can generate the backport patches for each stable release for
this patch series, so after this it will make the automated backporting
work.
I wonder how much value doing this for douts as opposed
to just pr_* messages actually brings?
I think the 'dout()' was introduced by printing more context info, which
includes module/function names and line#, when the
CONFIG_CEPH_LIB_PRETTYDEBUG is enabled.
Maybe we can remove CONFIG_CEPH_LIB_PRETTYDEBUG now, since the pr_* will
print the module name and also the caller for dout() and pr_* will print
the function name mostly ?
A regular user never sees douts as enabling them without extra care
tends to be useless -- journald can't cope with the volume and quickly
starts discarding them. From the developer side, many douts already
include at least one (hashed) pointer value which is usually sufficient
to disambiguate, even if it's more cumbersome than a grep on context
information.
Just in some cases the client global_id is really could help much,
especially for the issues only could be hit in productions case, which
could have hundreds of clients, and we can only enable some dout() debug
logs.
Thanks
- Xiubo
Thanks,
Ilya
