[ofa-general] Re: [PATCH] opensm/console: Fixed osm_console poll to so it won't eat up all CPU when stdin is a pipe

[Sasha Khapyorsky]

Hi Nicolas,

On 16:16 Mon 02 Mar     , Nicolas Morey Chaisemartin wrote:
> 
> Signed-off-by: Nicolas Morey-Chaisemartin <nicolas.morey-chaisemartin at ext.bull.net>
> ---

I guess that below is commit message, right? Assuming so, it should be
before '---' line.

> When opensm is called after a pipe to execute a command in the local console, it uses 100% CPU as osm_console poll returns as if there was something to read from stdin where there isn't.

As far as I understand described usage (echo blah-blah | opensm ....)
there should be nothing to read after 'echo' finished its job and poll()
will set POLLHUP revents bit.

> This patch fixes by closing the local console when the end of osm_console function has been reached, meaning poll returned a positive value but actually nothing to read. osm_console will return an error code which will unset the console_init_flag so it won't be reopened.
>
> For socket, the osm_console_close is called as it means the socket isn't in a proper state.

And then what osm_console() will do? Wouldn't it run over fd_in?

Sasha

> 
> 
>  opensm/include/opensm/osm_console.h |    2 +-
>  opensm/opensm/main.c                |    6 ++++--
>  opensm/opensm/osm_console.c         |   19 +++++++++++++++----
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/opensm/include/opensm/osm_console.h b/opensm/include/opensm/osm_console.h
> index 3ea8fa5..dcce1e4 100644
> --- a/opensm/include/opensm/osm_console.h
> +++ b/opensm/include/opensm/osm_console.h
> @@ -46,6 +46,6 @@
>  
>  BEGIN_C_DECLS
>  // TODO replace p_osm
> -void osm_console(osm_opensm_t * p_osm);
> +int osm_console(osm_opensm_t * p_osm);
>  END_C_DECLS
>  #endif				/* _OSM_CONSOLE_H_ */
> diff --git a/opensm/opensm/main.c b/opensm/opensm/main.c
> index 2271634..5a39f84 100644
> --- a/opensm/opensm/main.c
> +++ b/opensm/opensm/main.c
> @@ -494,8 +494,10 @@ int osm_manager_loop(osm_subn_opt_t * p_opt, osm_opensm_t * p_osm)
>  	   Sit here forever - dwell or do console i/o & cmds
>  	 */
>  	while (!osm_exit_flag) {
> -		if (console_init_flag)
> -			osm_console(p_osm);
> +		if (console_init_flag){
> +			if(osm_console(p_osm))
> +				console_init_flag = 0;
> +		}
>  		else
>  			cl_thread_suspend(10000);
>  
> diff --git a/opensm/opensm/osm_console.c b/opensm/opensm/osm_console.c
> index 63c5ea8..b6a8689 100644
> --- a/opensm/opensm/osm_console.c
> +++ b/opensm/opensm/osm_console.c
> @@ -1376,7 +1376,7 @@ static void parse_cmd_line(char *line, osm_opensm_t * p_osm)
>  	}
>  }
>  
> -void osm_console(osm_opensm_t * p_osm)
> +int osm_console(osm_opensm_t * p_osm)
>  {
>  	struct pollfd pollfd[2];
>  	char *p_line;
> @@ -1409,7 +1409,7 @@ void osm_console(osm_opensm_t * p_osm)
>  	}
>  
>  	if (poll(fds, nfds, 1000) <= 0)
> -		return;
> +		return 0;
>  
>  #ifdef ENABLE_OSM_CONSOLE_SOCKET
>  	if (pollfd[0].revents & POLLIN) {
> @@ -1422,7 +1422,7 @@ void osm_console(osm_opensm_t * p_osm)
>  				"ERR 4B04: Failed to accept console socket: %s\n",
>  				strerror(errno));
>  			p_oct->in_fd = -1;
> -			return;
> +			return 0;
>  		}
>  		if (inet_ntop
>  		    (AF_INET, &sin.sin_addr, p_oct->client_ip,
> @@ -1444,7 +1444,7 @@ void osm_console(osm_opensm_t * p_osm)
>  				p_oct->client_hn, p_oct->client_ip);
>  			close(new_fd);
>  		}
> -		return;
> +		return 0;
>  	}
>  #endif
>  
> @@ -1462,5 +1462,16 @@ void osm_console(osm_opensm_t * p_osm)
>  			osm_console_exit(p_oct, p_log);
>  		if (p_line)
>  			free(p_line);
> +		return 0;
>  	}
> +	/* If we are here, there has been a problem with poll
> +	 * so we exit the console not to eat up all the CPU
> +	 * if it was a local console
> +	 * If it was a socket, we just close it.
> +	 */
> +	if( p_oct->socket < 0 )
> +		return -1;
> +
> +	osm_console_exit(p_oct, p_log);
> +	return 0;
>  }
> -- 
> 1.5.6.3
>