From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753232AbeARCUP (ORCPT ); Wed, 17 Jan 2018 21:20:15 -0500 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:54623 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752901AbeARCUN (ORCPT ); Wed, 17 Jan 2018 21:20:13 -0500 X-ME-Sender: Subject: Re: [ANNOUNCE] autofs 5.1.2 release To: NeilBrown , autofs@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org References: <1465960537.3164.1.camel@themaw.net> <876092krew.fsf@notabene.neil.brown.name> <5aee2303-d1c3-49fd-0ef9-362fb0256fd5@themaw.net> <87po78khtj.fsf@notabene.neil.brown.name> From: Ian Kent Message-ID: <0db6bdd1-d096-ceeb-9f81-779af13a3c0d@themaw.net> Date: Thu, 18 Jan 2018 10:19:57 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <87po78khtj.fsf@notabene.neil.brown.name> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 21/12/17 09:09, NeilBrown wrote: > --------8<--------------- > Subject: use_hostname_for_mounts shouldn't prevent selection among replica > > If several replicas have been specified for a mount point, and > use_hostname_for_mount is set to "yes", the selection between > these replicas is currently disabled and the last in the list is always > chosen. > > There is little point selecting between different interfaces on the one > host in this case, but it is still worth selecting between different > hosts, particularly if different weights have been specified. > > This patch restores the "prune_host_list()" functionality when > use_hostname_for_mount is set, and modifies it slightly so that once > an IP address with a given proximity has been successfully probed, > other IP address for the same host(weight):/path and proximity are ignored. > > Signed-off-by: NeilBrown > > diff --git a/modules/replicated.c b/modules/replicated.c > index 3ac4c70f4062..16cf873513ff 100644 > --- a/modules/replicated.c > +++ b/modules/replicated.c > @@ -714,7 +714,7 @@ done: > int prune_host_list(unsigned logopt, struct host **list, > unsigned int vers, int port) > { > - struct host *this, *last, *first; > + struct host *this, *last, *first, *prev; > struct host *new = NULL; > unsigned int proximity, selected_version = 0; > unsigned int v2_tcp_count, v3_tcp_count, v4_tcp_count; > @@ -726,12 +726,6 @@ int prune_host_list(unsigned logopt, struct host **list, > if (!*list) > return 0; > > - /* If we're using the host name then there's no point probing > - * avialability and respose time. > - */ > - if (defaults_use_hostname_for_mounts()) > - return 1; > - > /* Use closest hosts to choose NFS version */ > > first = *list; > @@ -877,11 +871,18 @@ int prune_host_list(unsigned logopt, struct host **list, > > first = last; > this = first; > + prev = NULL; > while (this) { > struct host *next = this->next; > if (!this->name) { > remove_host(list, this); > add_host(&new, this); > + } else if (defaults_use_hostname_for_mounts() && prev && > + prev->proximity == this->proximity && > + strcmp(prev->name, this->name) == 0 && > + strcmp(prev->path, this->path) == 0 && > + prev->weight == this->weight) { > + /* No need to probe same host(weight):/path again */ Mmm ... so maybe I'm the one that's missing the point. You are trying to eliminate multiple occurrences of list entries that correspond to a specific host name entry from probing. It might be sensible to add a "this->rr" following the defaults_use_hostname_for_mounts() check to avoid the additional checks when the host doesn't have additional addresses, particularly the string comparison. There's nothing stopping people from adding this same host name with a different weight, even though that doesn't seem like a sensible thing to do. I'm not sure if this exposes mounting to problems that aren't already present with the current implementation. I'll think a little more about that case but at first glance the DNS round robin problem of addresses referring to different devices is still present, a possible false negative. But that problem exits in the current implementation too as a round robin lookup can just as easily return an address of a host that isn't responding at mount time..... > } else { > status = get_supported_ver_and_cost(logopt, this, > selected_version, port); > @@ -889,6 +890,7 @@ int prune_host_list(unsigned logopt, struct host **list, > this->version = selected_version; > remove_host(list, this); > add_host(&new, this); > + prev = this; > } > } > this = next; >